commit 43144b15d3e8002c6add5c3af40fd5a745f8c563
author Will McVicker <willmcvicker@google.com> Tue Apr 13 22:59:41 2021
committer Commit Bot <commit-bot@chromium.org> Wed May 12 23:05:05 2021
tree a7b9d835a55d20441988df7d7ec2fbb79d611782
parent efe47db10eb68fd9c0304f5b11296a6f22c3d0ed

BACKPORT: FROMGIT: ANDROID: xt_qtaguid: fix UAF race

Make sure to hold the sock_tag_list_lock while accessing the tag to
avoid a race between getting the tag and free'ing the tag.

Bug: 184018316
Disallow-Recycled-Builds: test-failures
Fixes: c7ca0ac69702 ("ANDROID: netfilter: xt_qtaguid: add qtaguid matching module")
Signed-off-by: Will McVicker <willmcvicker@google.com>

(cherry picked from commit 2398e650c58a6f4877dafce649188290f6e3b4f5
 https://android.googlesource.com/kernel/common android-4.14-p)

Conflicts:
   net/netfilter/xt_qtaguid.c
   Locking in ChromeOS is per network namespace.

BUG=chromium:1195431
TEST=Run PoC

Change-Id: If50e33958b982c41f0fa9e9bbb02ba33f673b83a
Signed-off-by: Guenter Roeck <groeck@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2884362
Reviewed-by: Zubin Mithra <zsm@chromium.org>
Commit-Queue: Guenter Roeck <groeck@chromium.org>
Tested-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit a5966bd58466c4292297c81d75bcbb3a9e690474)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2889675
Reviewed-by: Guenter Roeck <groeck@chromium.org>

net/netfilter/xt_qtaguid.c

diff --git a/net/netfilter/xt_qtaguid.c b/net/netfilter/xt_qtaguid.c
index 2e12fdb..5070c95b 100644
--- a/net/netfilter/xt_qtaguid.c
+++ b/net/netfilter/xt_qtaguid.c
@@ -1176,19 +1176,6 @@
 	return sock_tag_tree_search(&qtaguid_net->sock_tag_tree, sk);
 }
 
-static struct sock_tag *get_sock_stat(struct qtaguid_net *qtaguid_net,
-				      const struct sock *sk)
-{
-	struct sock_tag *sock_tag_entry;
-	MT_DEBUG("qtaguid: get_sock_stat(sk=%p)\n", sk);
-	if (!sk)
-		return NULL;
-	spin_lock_bh(&qtaguid_net->sock_tag_list_lock);
-	sock_tag_entry = get_sock_stat_nl(qtaguid_net, sk);
-	spin_unlock_bh(&qtaguid_net->sock_tag_list_lock);
-	return sock_tag_entry;
-}
-
 static int ipx_proto(const struct sk_buff *skb,
 		     struct xt_action_param *par)
 {
@@ -1429,12 +1416,15 @@
 	 * Look for a tagged sock.
 	 * It will have an acct_uid.
 	 */
-	sock_tag_entry = get_sock_stat(qtaguid_net, sk);
+	spin_lock_bh(&qtaguid_net->sock_tag_list_lock);
+	sock_tag_entry = sk ? get_sock_stat_nl(qtaguid_net, sk) : NULL;
 	if (sock_tag_entry) {
 		tag = sock_tag_entry->tag;
 		acct_tag = get_atag_from_tag(tag);
 		uid_tag = get_utag_from_tag(tag);
-	} else {
+	}
+	spin_unlock_bh(&qtaguid_net->sock_tag_list_lock);
+	if (!sock_tag_entry) {
 		acct_tag = make_atag_from_value(0);
 		tag = combine_atag_with_uid(acct_tag, uid);
 		uid_tag = make_tag_from_uid(uid);