netdev - [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240724152149.11003-1-aha310510@gmail.com>
Date: Thu, 25 Jul 2024 00:21:49 +0900
From: Jeongjun Park <aha310510@...il.com>
To: willemdebruijn.kernel@...il.com,
	jasowang@...hat.com
Cc: syzbot+44623300f057a28baf1e@...kaller.appspotmail.com,
	davem@...emloft.net,
	edumazet@...gle.com,
	pabeni@...hat.com,
	kuba@...nel.org,
	jiri@...nulli.us,
	bigeasy@...utronix.de,
	linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org,
	bpf@...r.kernel.org,
	syzkaller-bugs@...glegroups.com,
	Jeongjun Park <aha310510@...il.com>
Subject: [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp_generic()

In the previous commit, bpf_net_context handling was added to 
tun_sendmsg() and do_xdp_generic(), but if you write code like this,
bpf_net_context overlaps in the call trace below, causing various
memory corruptions.

<Call trace>
...
tun_sendmsg() // bpf_net_ctx_set()
  tun_xdp_one()
    do_xdp_generic() // bpf_net_ctx_set() <-- nested
...

This patch removes the bpf_net_context handling that exists in 
do_xdp_generic() and modifies it to handle it in the parent function.

Reported-by: syzbot+44623300f057a28baf1e@...kaller.appspotmail.com
Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
Signed-off-by: Jeongjun Park <aha310510@...il.com>
---
 drivers/net/tun.c | 3 +++
 net/core/dev.c    | 8 +++-----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9b24861464bc..095ada4a525e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1919,10 +1919,12 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 
 	if (skb_xdp) {
 		struct bpf_prog *xdp_prog;
+		struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
 		int ret;
 
 		local_bh_disable();
 		rcu_read_lock();
+		bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);
 		xdp_prog = rcu_dereference(tun->xdp_prog);
 		if (xdp_prog) {
 			ret = do_xdp_generic(xdp_prog, &skb);
@@ -1932,6 +1934,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 				goto unlock_frags;
 			}
 		}
+		bpf_net_ctx_clear(bpf_net_ctx);
 		rcu_read_unlock();
 		local_bh_enable();
 	}
diff --git a/net/core/dev.c b/net/core/dev.c
index 6ea1d20676fb..26f9fdd66e64 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5126,14 +5126,11 @@ static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
 
 int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff **pskb)
 {
-	struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
-
 	if (xdp_prog) {
 		struct xdp_buff xdp;
 		u32 act;
 		int err;
 
-		bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);
 		act = netif_receive_generic_xdp(pskb, &xdp, xdp_prog);
 		if (act != XDP_PASS) {
 			switch (act) {
@@ -5147,13 +5144,11 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff **pskb)
 				generic_xdp_tx(*pskb, xdp_prog);
 				break;
 			}
-			bpf_net_ctx_clear(bpf_net_ctx);
 			return XDP_DROP;
 		}
 	}
 	return XDP_PASS;
 out_redir:
-	bpf_net_ctx_clear(bpf_net_ctx);
 	kfree_skb_reason(*pskb, SKB_DROP_REASON_XDP);
 	return XDP_DROP;
 }
@@ -5475,10 +5470,13 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
 
 	if (static_branch_unlikely(&generic_xdp_needed_key)) {
 		int ret2;
+		struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
 
 		migrate_disable();
+		bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);
 		ret2 = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog),
 				      &skb);
+		bpf_net_ctx_clear(bpf_net_ctx);
 		migrate_enable();
 
 		if (ret2 != XDP_PASS) {
--