[syzbot] [net?] [bpf?] general protection fault in __dev_flush

[syzbot] [net?] [bpf?] general protection fault in __dev_flush

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    68b59730459e Merge tag 'perf-tools-for-v6.11-2024-07-16' o..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14cb0ab5980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b6230d83d52af231
dashboard link: https://syzkaller.appspot.com/bug?extid=44623300f057a28baf1e
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

Unfortunately, I don't have any reproducer for this issue yet.

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/8229997a3dbb/disk-68b59730.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/fd51823e0836/vmlinux-68b59730.xz
kernel image: https://storage.googleapis.com/syzbot-assets/01811b27f987/bzImage-68b59730.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com

Oops: general protection fault, probably for non-canonical address 0xfbd5a5d5a0000001: 0000 [#1] PREEMPT SMP KASAN NOPTI
KASAN: maybe wild-memory-access in range [0xdead4ead00000008-0xdead4ead0000000f]
CPU: 1 PID: 8860 Comm: syz.0.1070 Not tainted 6.10.0-syzkaller-08280-g68b59730459e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/27/2024
RIP: 0010:__list_del include/linux/list.h:195 [inline]
RIP: 0010:__list_del_clearprev include/linux/list.h:209 [inline]
RIP: 0010:__dev_flush+0xe4/0x160 kernel/bpf/devmap.c:428
Code: b8 00 00 00 00 00 fc ff df 41 80 7c 05 00 00 49 89 c5 74 08 48 89 df e8 6a c3 3d 00 48 8b 2b 48 8d 5d 08 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 3d c4 3d 00 4c 89 23 4c 89 e0 48
RSP: 0018:ffffc90000a18af0 EFLAGS: 00010212
RAX: 1bd5a9d5a0000001 RBX: dead4ead00000008 RCX: 0000000000000000
RDX: 0000000000000010 RSI: 0000000000000000 RDI: ffff8880b943e868
RBP: dead4ead00000000 R08: ffff8880b943e867 R09: ffff8880b943e858
R10: dffffc0000000000 R11: ffffed1017287d0d R12: 00000000ffffffff
R13: dffffc0000000000 R14: ffff8880b943e848 R15: 1ffff11017287d09
FS:  00007f33633c96c0(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffec0000 CR3: 0000000054f40000 CR4: 0000000000350ef0
Call Trace:
 <IRQ>
 xdp_do_check_flushed+0x129/0x240 net/core/filter.c:4300
 __napi_poll+0xe4/0x490 net/core/dev.c:6774
 napi_poll net/core/dev.c:6840 [inline]
 net_rx_action+0x89b/0x1240 net/core/dev.c:6962
 handle_softirqs+0x2c6/0x970 kernel/softirq.c:554
 __do_softirq kernel/softirq.c:588 [inline]
 invoke_softirq kernel/softirq.c:428 [inline]
 __irq_exit_rcu+0xf4/0x1c0 kernel/softirq.c:637
 irq_exit_rcu+0x9/0x30 kernel/softirq.c:649
 common_interrupt+0xaa/0xd0 arch/x86/kernel/irq.c:278
 </IRQ>
 <TASK>
 asm_common_interrupt+0x26/0x40 arch/x86/include/asm/idtentry.h:693
RIP: 0010:finish_task_switch+0x1ea/0x870 kernel/sched/core.c:5062
Code: c9 50 e8 19 b6 0b 00 48 83 c4 08 4c 89 f7 e8 7d 38 00 00 e9 de 04 00 00 4c 89 f7 e8 d0 d9 32 0a e8 4b e8 36 00 fb 48 8b 5d c0 <48> 8d bb f8 15 00 00 48 89 f8 48 c1 e8 03 49 be 00 00 00 00 00 fc
RSP: 0018:ffffc90003a377a8 EFLAGS: 00000286
RAX: 94ed15acce52c200 RBX: ffff88807e01da00 RCX: ffffffff947db703
RDX: dffffc0000000000 RSI: ffffffff8bcac9a0 RDI: ffffffff8c205b20
RBP: ffffc90003a377f0 R08: ffffffff8faec7af R09: 1ffffffff1f5d8f5
R10: dffffc0000000000 R11: fffffbfff1f5d8f6 R12: 1ffff110172a7ebb
R13: dffffc0000000000 R14: ffff8880b943e840 R15: ffff8880b953f5d8
 context_switch kernel/sched/core.c:5191 [inline]
 __schedule+0x1808/0x4a60 kernel/sched/core.c:6529
 __schedule_loop kernel/sched/core.c:6606 [inline]
 schedule+0x14b/0x320 kernel/sched/core.c:6621
 futex_wait_queue+0x14e/0x1d0 kernel/futex/waitwake.c:370
 __futex_wait+0x17f/0x320 kernel/futex/waitwake.c:669
 futex_wait+0x101/0x360 kernel/futex/waitwake.c:697
 do_futex+0x33b/0x560 kernel/futex/syscalls.c:102
 __do_sys_futex kernel/futex/syscalls.c:179 [inline]
 __se_sys_futex+0x3f9/0x480 kernel/futex/syscalls.c:160
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f3362575b59
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f33633c90f8 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: ffffffffffffffda RBX: 00007f3362705f68 RCX: 00007f3362575b59
RDX: 0000000000000000 RSI: 0000000000000080 RDI: 00007f3362705f68
RBP: 00007f3362705f60 R08: 00007f33633c96c0 R09: 00007f33633c96c0
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3362705f6c
R13: 000000000000000b R14: 00007ffec9a21080 R15: 00007ffec9a21168
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:__list_del include/linux/list.h:195 [inline]
RIP: 0010:__list_del_clearprev include/linux/list.h:209 [inline]
RIP: 0010:__dev_flush+0xe4/0x160 kernel/bpf/devmap.c:428
Code: b8 00 00 00 00 00 fc ff df 41 80 7c 05 00 00 49 89 c5 74 08 48 89 df e8 6a c3 3d 00 48 8b 2b 48 8d 5d 08 48 89 d8 48 c1 e8 03 <42> 80 3c 28 00 74 08 48 89 df e8 3d c4 3d 00 4c 89 23 4c 89 e0 48
RSP: 0018:ffffc90000a18af0 EFLAGS: 00010212
RAX: 1bd5a9d5a0000001 RBX: dead4ead00000008 RCX: 0000000000000000
RDX: 0000000000000010 RSI: 0000000000000000 RDI: ffff8880b943e868
RBP: dead4ead00000000 R08: ffff8880b943e867 R09: ffff8880b943e858
R10: dffffc0000000000 R11: ffffed1017287d0d R12: 00000000ffffffff
R13: dffffc0000000000 R14: ffff8880b943e848 R15: 1ffff11017287d09
FS:  00007f33633c96c0(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffec0000 CR3: 0000000054f40000 CR4: 0000000000350ef0
----------------
Code disassembly (best guess):
   0:	b8 00 00 00 00       	mov    $0x0,%eax
   5:	00 fc                	add    %bh,%ah
   7:	ff                   	(bad)
   8:	df 41 80             	filds  -0x80(%rcx)
   b:	7c 05                	jl     0x12
   d:	00 00                	add    %al,(%rax)
   f:	49 89 c5             	mov    %rax,%r13
  12:	74 08                	je     0x1c
  14:	48 89 df             	mov    %rbx,%rdi
  17:	e8 6a c3 3d 00       	call   0x3dc386
  1c:	48 8b 2b             	mov    (%rbx),%rbp
  1f:	48 8d 5d 08          	lea    0x8(%rbp),%rbx
  23:	48 89 d8             	mov    %rbx,%rax
  26:	48 c1 e8 03          	shr    $0x3,%rax
* 2a:	42 80 3c 28 00       	cmpb   $0x0,(%rax,%r13,1) <-- trapping instruction
  2f:	74 08                	je     0x39
  31:	48 89 df             	mov    %rbx,%rdi
  34:	e8 3d c4 3d 00       	call   0x3dc476
  39:	4c 89 23             	mov    %r12,(%rbx)
  3c:	4c 89 e0             	mov    %r12,%rax
  3f:	48                   	rex.W


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [bpf?] [net?] general protection fault in __dev_flush

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit:    7846b618e0a4 Merge tag 'rtc-6.11' of git://git.kernel.org/..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=142d3eb5980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=be4129de17851dbe
dashboard link: https://syzkaller.appspot.com/bug?extid=44623300f057a28baf1e
compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=154c40b1980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14f3e11d980000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-7846b618.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/3a2831ffe61c/vmlinux-7846b618.xz
kernel image: https://storage.googleapis.com/syzbot-assets/575e23a7c452/bzImage-7846b618.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
CPU: 1 PID: 5389 Comm: syz-executor357 Not tainted 6.10.0-syzkaller-11323-g7846b618e0a4 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:__dev_flush+0x49/0x1e0 kernel/bpf/devmap.c:424
Code: 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 98 01 00 00 48 b8 00 00 00 00 00 fc ff df 49 8b 2f 48 8d 5d 80 48 89 ea 48 c1 ea 03 <80> 3c 02 00 0f 85 69 01 00 00 48 8b 45 00 49 39 ef 4c 8d 60 80 0f
RSP: 0018:ffffc900008b0c90 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: ffffffffffffff80 RCX: ffffffff88d6a5bb
RDX: 0000000000000000 RSI: ffffffff81af9c56 RDI: ffffc9000345fa68
RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffc9000345fa58
R13: ffff888022ec0fb0 R14: ffffc9000345fa68 R15: ffffc9000345fa68
FS:  0000555568ea6380(0000) GS:ffff88806b100000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ff4743880f0 CR3: 0000000023168000 CR4: 0000000000350ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <IRQ>
 xdp_do_check_flushed+0x40a/0x4e0 net/core/filter.c:4300
 __napi_poll.constprop.0+0xd1/0x550 net/core/dev.c:6774
 napi_poll net/core/dev.c:6840 [inline]
 net_rx_action+0xa92/0x1010 net/core/dev.c:6962
 handle_softirqs+0x216/0x8f0 kernel/softirq.c:554
 do_softirq kernel/softirq.c:455 [inline]
 do_softirq+0xb2/0xf0 kernel/softirq.c:442
 </IRQ>
 <TASK>
 __local_bh_enable_ip+0x100/0x120 kernel/softirq.c:382
 local_bh_enable include/linux/bottom_half.h:33 [inline]
 tun_get_user+0x1d9b/0x3c30 drivers/net/tun.c:1936
 tun_chr_write_iter+0xe8/0x210 drivers/net/tun.c:2052
 new_sync_write fs/read_write.c:497 [inline]
 vfs_write+0x6b6/0x1140 fs/read_write.c:590
 ksys_write+0x12f/0x260 fs/read_write.c:643
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7ff47430af50
Code: 40 00 48 c7 c2 b8 ff ff ff f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 80 3d 51 e1 07 00 00 74 17 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 48 83 ec 28 48 89
RSP: 002b:00007ffde0326728 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007ffde03267c0 RCX: 00007ff47430af50
RDX: 0000000000000e80 RSI: 0000000020000100 RDI: 00000000000000c8
RBP: 00007ffde0326770 R08: 00007ffde0326750 R09: 00007ffde0326750
R10: 00007ffde0326750 R11: 0000000000000202 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:__dev_flush+0x49/0x1e0 kernel/bpf/devmap.c:424
Code: 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 98 01 00 00 48 b8 00 00 00 00 00 fc ff df 49 8b 2f 48 8d 5d 80 48 89 ea 48 c1 ea 03 <80> 3c 02 00 0f 85 69 01 00 00 48 8b 45 00 49 39 ef 4c 8d 60 80 0f
RSP: 0018:ffffc900008b0c90 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: ffffffffffffff80 RCX: ffffffff88d6a5bb
RDX: 0000000000000000 RSI: ffffffff81af9c56 RDI: ffffc9000345fa68
RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffc9000345fa58
R13: ffff888022ec0fb0 R14: ffffc9000345fa68 R15: ffffc9000345fa68
FS:  0000555568ea6380(0000) GS:ffff88806b100000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ff4743880f0 CR3: 0000000023168000 CR4: 0000000000350ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess), 4 bytes skipped:
   0:	48 c1 ea 03          	shr    $0x3,%rdx
   4:	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1)
   8:	0f 85 98 01 00 00    	jne    0x1a6
   e:	48 b8 00 00 00 00 00 	movabs $0xdffffc0000000000,%rax
  15:	fc ff df
  18:	49 8b 2f             	mov    (%r15),%rbp
  1b:	48 8d 5d 80          	lea    -0x80(%rbp),%rbx
  1f:	48 89 ea             	mov    %rbp,%rdx
  22:	48 c1 ea 03          	shr    $0x3,%rdx
* 26:	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1) <-- trapping instruction
  2a:	0f 85 69 01 00 00    	jne    0x199
  30:	48 8b 45 00          	mov    0x0(%rbp),%rax
  34:	49 39 ef             	cmp    %rbp,%r15
  37:	4c 8d 60 80          	lea    -0x80(%rax),%r12
  3b:	0f                   	.byte 0xf


---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

[PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp_generic()

[Jeongjun Park]

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@syzkaller.appspotmail.com
Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
Signed-off-by: Jeongjun Park <aha310510@gmail.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) {
--

Re: [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp_generic()

[Willem de Bruijn]

Jeongjun Park wrote:
> 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.

I'm no expert on this code, but commit 401cb7dae813 that introduced
bpf_net_ctx_set explicitly states that nested calls are allowed.

And the function does imply that:

static inline struct bpf_net_context *bpf_net_ctx_set(struct bpf_net_context *bpf_net_ctx)
{
        struct task_struct *tsk = current;

        if (tsk->bpf_net_context != NULL)
                return NULL;
        bpf_net_ctx->ri.kern_flags = 0;

        tsk->bpf_net_context = bpf_net_ctx;
        return bpf_net_ctx;
}


 
> <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.

Is tun_xdp_one missing? That also calls do_xdp_generic.

Re: [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp_generic()

[Jeongjun Park]

Willem de Bruijn wrote:
> I'm no expert on this code, but commit 401cb7dae813 that introduced
> bpf_net_ctx_set explicitly states that nested calls are allowed.
>
> And the function does imply that:
>
> static inline struct bpf_net_context *bpf_net_ctx_set(struct bpf_net_context *bpf_net_ctx)
> {
>         struct task_struct *tsk = current;
>
>         if (tsk->bpf_net_context != NULL)
>                 return NULL;
>         bpf_net_ctx->ri.kern_flags = 0;
>
>         tsk->bpf_net_context = bpf_net_ctx;
>         return bpf_net_ctx;
> }

I'm not an expert on this code either. As you said, there is a 
possibility that the bug is not caused by overlapping calls, but various
memory corruptions are occurring due to the handling of bpf_net_context 
in do_xdp_generic. Therefore, it is appropriate to modify it to handle
it in the parent function rather than in do_xdp_generic.

> Is tun_xdp_one missing? That also calls do_xdp_generic.

This is no problem since tun_xdp_one is only called from tun_sendmsg 
and tun_sendmsg already does the bpf_net_context handling.

Regards,
Jeongjun Park.

Re: [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp_generic()

[Paolo Abeni]

On 7/25/24 04:43, Willem de Bruijn wrote:
> Jeongjun Park wrote:
>> 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.
> 
> I'm no expert on this code, but commit 401cb7dae813 that introduced
> bpf_net_ctx_set explicitly states that nested calls are allowed.
> 
> And the function does imply that:
> 
> static inline struct bpf_net_context *bpf_net_ctx_set(struct bpf_net_context *bpf_net_ctx)
> {
>          struct task_struct *tsk = current;
> 
>          if (tsk->bpf_net_context != NULL)
>                  return NULL;
>          bpf_net_ctx->ri.kern_flags = 0;
> 
>          tsk->bpf_net_context = bpf_net_ctx;
>          return bpf_net_ctx;
> }

I agree with Willem, the ctx nesting looks legit generally speaking. 
@Jeongjun: you need to track down more accurately the issue root cause 
and include such info into the commit message.

Skimming over the code I *think* do_xdp_generic() is not cleaning the 
nested context in all the paths before return and that could cause the 
reported issue.

Thanks,

Paolo

Re: [PATCH net] tun: Remove nested call to bpf_net_ctx_set() in do_xdp_generic()

[Jeongjun Park]

Paolo Abeni wrote:
>
> On 7/25/24 04:43, Willem de Bruijn wrote:
> > Jeongjun Park wrote:
> >> 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.
> >
> > I'm no expert on this code, but commit 401cb7dae813 that introduced
> > bpf_net_ctx_set explicitly states that nested calls are allowed.
> >
> > And the function does imply that:
> >
> > static inline struct bpf_net_context *bpf_net_ctx_set(struct bpf_net_context *bpf_net_ctx)
> > {
> >          struct task_struct *tsk = current;
> >
> >          if (tsk->bpf_net_context != NULL)
> >                  return NULL;
> >          bpf_net_ctx->ri.kern_flags = 0;
> >
> >          tsk->bpf_net_context = bpf_net_ctx;
> >          return bpf_net_ctx;
> > }
>
> I agree with Willem, the ctx nesting looks legit generally speaking.
> @Jeongjun: you need to track down more accurately the issue root cause
> and include such info into the commit message.
>
> Skimming over the code I *think* do_xdp_generic() is not cleaning the
> nested context in all the paths before return and that could cause the
> reported issue.

Thanks to your comment, I re-read the code and found the root cause.
I will send a patch for that bug.

Regards,
Jeongjun Park

[PATCH net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()

[Jeongjun Park]

There are cases where do_xdp_generic returns bpf_net_context without 
clearing it. This causes various memory corruptions, so the missing 
bpf_net_ctx_clear must be added.

Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com
Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 net/core/dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6ea1d20676fb..751d9b70e6ad 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5150,6 +5150,7 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff **pskb)
 			bpf_net_ctx_clear(bpf_net_ctx);
 			return XDP_DROP;
 		}
+		bpf_net_ctx_clear(bpf_net_ctx);
 	}
 	return XDP_PASS;
 out_redir:
--

Re: [PATCH net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()

[Jason Wang]

On Fri, Jul 26, 2024 at 5:41 AM Jeongjun Park <aha310510@gmail.com> wrote:
>
> There are cases where do_xdp_generic returns bpf_net_context without
> clearing it. This causes various memory corruptions, so the missing
> bpf_net_ctx_clear must be added.
>
> Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com
> Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>

Acked-by: Jason Wang <jasowang@redhat.com>

(Looks like the do_xdp_generic() needs some tweak for example we can
merge the two paths for XDP_DROP at least).

Thanks

> ---
>  net/core/dev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6ea1d20676fb..751d9b70e6ad 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5150,6 +5150,7 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff **pskb)
>                         bpf_net_ctx_clear(bpf_net_ctx);
>                         return XDP_DROP;
>                 }
> +               bpf_net_ctx_clear(bpf_net_ctx);
>         }
>         return XDP_PASS;
>  out_redir:
> --
>

Re: [PATCH net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()

[Willem de Bruijn]

On Thu, Jul 25, 2024 at 10:21 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Jul 26, 2024 at 5:41 AM Jeongjun Park <aha310510@gmail.com> wrote:
> >
> > There are cases where do_xdp_generic returns bpf_net_context without
> > clearing it. This causes various memory corruptions, so the missing
> > bpf_net_ctx_clear must be added.
> >
> > Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com
> > Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
> > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
>
> Acked-by: Jason Wang <jasowang@redhat.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()

[Jakub Kicinski]

On Fri, 26 Jul 2024 06:40:49 +0900 Jeongjun Park wrote:
> There are cases where do_xdp_generic returns bpf_net_context without 
> clearing it. This causes various memory corruptions, so the missing 
> bpf_net_ctx_clear must be added.
> 
> Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com
> Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>

Also likely:

Reported-by: syzbot+3c2b6d5d4bec3b904933@syzkaller.appspotmail.com
Reported-by: syzbot+707d98c8649695eaf329@syzkaller.appspotmail.com

Right?

[PATCH net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()

[Jeongjun Park]

Jakub Kicinski wrote:
>
> On Fri, 26 Jul 2024 06:40:49 +0900 Jeongjun Park wrote:
> > There are cases where do_xdp_generic returns bpf_net_context without
> > clearing it. This causes various memory corruptions, so the missing
> > bpf_net_ctx_clear must be added.
> >
> > Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com
> > Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
> > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
>
> Also likely:
>
> Reported-by: syzbot+3c2b6d5d4bec3b904933@syzkaller.appspotmail.com
> Reported-by: syzbot+707d98c8649695eaf329@syzkaller.appspotmail.com
>
> Right?

Yes, both appear to be bugs with the same root cause.

Regards,
Jeongjun Park

Re: [PATCH net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()

[Jeongjun Park]

On Fri, 26 Jul 2024 06:40:49 +0900 Jeongjun Park wrote:
> There are cases where do_xdp_generic returns bpf_net_context without
> clearing it. This causes various memory corruptions, so the missing
> bpf_net_ctx_clear must be added.
>
> Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com
> Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>

Reported-by: syzbot+c226757eb784a9da3e8b@syzkaller.appspotmail.com
Reported-by: syzbot+61a1cfc2b6632363d319@syzkaller.appspotmail.com
Reported-by: syzbot+709e4c85c904bcd62735@syzkaller.appspotmail.com

After searching, I found reports with the same root cause, so I added
them.

Re: [PATCH net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 26 Jul 2024 06:40:49 +0900 you wrote:
> There are cases where do_xdp_generic returns bpf_net_context without
> clearing it. This causes various memory corruptions, so the missing
> bpf_net_ctx_clear must be added.
> 
> Reported-by: syzbot+44623300f057a28baf1e@syzkaller.appspotmail.com
> Fixes: fecef4cd42c6 ("tun: Assign missing bpf_net_context.")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> 
> [...]

Here is the summary with links:
  - [net] tun: Add missing bpf_net_ctx_clear() in do_xdp_generic()
    https://git.kernel.org/netdev/net/c/9da49aa80d68

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html