| Message ID | 20230721233330.5678-1-daniel@iogearbox.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | dc644b540a2d2874112706591234be3d3fbf9ef7 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] tcx: Fix splat in ingress_destroy upon tcx_entry_free | expand |
On 7/22/23 1:33 AM, Daniel Borkmann wrote: > On qdisc destruction, the ingress_destroy() needs to update the correct > entry, that is, tcx_entry_update must NULL the dev->tcx_ingress pointer. > Therefore, fix the typo. > > Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support") > Reported-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com > Reported-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com > Reported-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Additionally, syzbot also tested that all 3 reports are fixed with this: Tested-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com Tested-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com Tested-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com
Daniel Borkmann <daniel@iogearbox.net> writes: > On qdisc destruction, the ingress_destroy() needs to update the correct > entry, that is, tcx_entry_update must NULL the dev->tcx_ingress pointer. > Therefore, fix the typo. > > Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support") > Reported-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com > Reported-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com > Reported-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Thanks! Tested-by: Petr Machata <petrm@nvidia.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Sat, 22 Jul 2023 01:33:30 +0200 you wrote: > On qdisc destruction, the ingress_destroy() needs to update the correct > entry, that is, tcx_entry_update must NULL the dev->tcx_ingress pointer. > Therefore, fix the typo. > > Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support") > Reported-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com > Reported-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com > Reported-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > > [...] Here is the summary with links: - [net-next] tcx: Fix splat in ingress_destroy upon tcx_entry_free https://git.kernel.org/netdev/net-next/c/dc644b540a2d You are awesome, thank you!
On 22/07/2023 2:33, Daniel Borkmann wrote: > On qdisc destruction, the ingress_destroy() needs to update the correct > entry, that is, tcx_entry_update must NULL the dev->tcx_ingress pointer. > Therefore, fix the typo. > > Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support") > Reported-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com > Reported-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com > Reported-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Hi Daniel, Our nightly regression testing picked up new memory leaks which were bisected to this commit. Unfortunately, I do not know the exact repro steps to trigger it, maybe the attached kmemeleak logs can help? unreferenced object 0xffff88811ce37b80 (size 224): comm "kworker/14:1", pid 7451, jiffies 4295350041 (age 64.444s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 e0 69 29 81 88 ff ff 00 3a 39 0d 81 88 ff ff ..i).....:9..... backtrace: [<00000000a0f098fe>] __alloc_skb+0x1f4/0x2b0 [<000000000dabee54>] alloc_skb_with_frags+0x7a/0x6c0 [<00000000e681c78a>] sock_alloc_send_pskb+0x63f/0x7d0 [<00000000a4774143>] mld_newpack.isra.0+0x1ad/0x800 [<0000000060e32100>] add_grhead+0x271/0x320 [<00000000040e7099>] add_grec+0xc8b/0x1120 [<000000009853483c>] mld_ifc_work+0x387/0xae0 [<0000000079d8299d>] process_one_work+0x86a/0x1430 [<000000001968010b>] worker_thread+0x5b0/0xf00 [<0000000090c285b0>] kthread+0x2dd/0x3b0 [<000000001f322d79>] ret_from_fork+0x2d/0x70 [<000000008ad6bd7b>] ret_from_fork_asm+0x11/0x20 unreferenced object 0xffff888153058640 (size 224): comm "softirq", pid 0, jiffies 4295350849 (age 61.212s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 e0 69 29 81 88 ff ff c0 32 39 0d 81 88 ff ff ..i).....29..... backtrace: [<00000000a0f098fe>] __alloc_skb+0x1f4/0x2b0 [<00000000bb2ddb4c>] ndisc_alloc_skb+0x133/0x340 [<0000000009614816>] ndisc_send_rs+0x1e0/0x4b0 [<000000004bc1b8be>] addrconf_rs_timer+0x25a/0x720 [<000000004d021706>] call_timer_fn+0x167/0x3d0 [<0000000088aa76a3>] __run_timers.part.0+0x546/0x8b0 [<0000000066f62ff3>] run_timer_softirq+0x6a/0x100 [<000000003732ddfb>] __do_softirq+0x264/0x80c unreferenced object 0xffff888155d0a500 (size 224): comm "softirq", pid 0, jiffies 4295352832 (age 53.328s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 e0 69 29 81 88 ff ff c0 32 39 0d 81 88 ff ff ..i).....29..... backtrace: [<00000000a0f098fe>] __alloc_skb+0x1f4/0x2b0 [<00000000bb2ddb4c>] ndisc_alloc_skb+0x133/0x340 [<0000000009614816>] ndisc_send_rs+0x1e0/0x4b0 [<000000004bc1b8be>] addrconf_rs_timer+0x25a/0x720 [<000000004d021706>] call_timer_fn+0x167/0x3d0 [<0000000088aa76a3>] __run_timers.part.0+0x546/0x8b0 [<0000000066f62ff3>] run_timer_softirq+0x6a/0x100 [<000000003732ddfb>] __do_softirq+0x264/0x80c unreferenced object 0xffff88814e3f6040 (size 576): comm "softirq", pid 0, jiffies 4295352832 (age 53.328s) hex dump (first 32 bytes): 00 00 33 33 00 00 00 02 e8 eb d3 98 21 bc 86 dd ..33........!... 60 00 00 00 00 10 3a ff fe 80 00 00 00 00 00 00 `.....:......... backtrace: [<00000000525ad98b>] kmalloc_reserve+0x118/0x1f0 [<000000008d146183>] __alloc_skb+0x105/0x2b0 [<00000000bb2ddb4c>] ndisc_alloc_skb+0x133/0x340 [<0000000009614816>] ndisc_send_rs+0x1e0/0x4b0 [<000000004bc1b8be>] addrconf_rs_timer+0x25a/0x720 [<000000004d021706>] call_timer_fn+0x167/0x3d0 [<0000000088aa76a3>] __run_timers.part.0+0x546/0x8b0 [<0000000066f62ff3>] run_timer_softirq+0x6a/0x100 [<000000003732ddfb>] __do_softirq+0x264/0x80c unreferenced object 0xffff88812acdebc0 (size 16): comm "umount.nfs", pid 11626, jiffies 4295354796 (age 45.472s) hex dump (first 16 bytes): 73 65 72 76 65 72 2d 32 00 eb cd 2a 81 88 ff ff server-2...*.... backtrace: [<0000000010fb5130>] __kmalloc_node_track_caller+0x4c/0x170 [<00000000b866a733>] kvasprintf+0xb0/0x130 [<00000000b3564fca>] kasprintf+0xa6/0xd0 [<00000000f01d6cb3>] nfs_sysfs_move_sb_to_server+0x49/0xd0 [<000000009608708f>] nfs_kill_super+0x5f/0x90 [<0000000090d4108b>] deactivate_locked_super+0x80/0x130 [<000000000856aeb1>] cleanup_mnt+0x258/0x370 [<0000000040582e39>] task_work_run+0x12c/0x210 [<00000000378ea041>] exit_to_user_mode_prepare+0x1a0/0x1b0 [<00000000025e63dd>] syscall_exit_to_user_mode+0x19/0x50 [<00000000f34ad3ee>] do_syscall_64+0x4a/0x90 [<000000009d3e2403>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
On 8/3/23 4:10 AM, Gal Pressman wrote: > On 22/07/2023 2:33, Daniel Borkmann wrote: >> On qdisc destruction, the ingress_destroy() needs to update the correct >> entry, that is, tcx_entry_update must NULL the dev->tcx_ingress pointer. >> Therefore, fix the typo. >> >> Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support") >> Reported-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com >> Reported-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com >> Reported-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com >> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > > Hi Daniel, > > Our nightly regression testing picked up new memory leaks which were > bisected to this commit. Thanks for the report. Can you help to check if it can be reproduced with the latest net-next which has a tcx fix in commit 079082c60aff ("tcx: Fix splat during dev unregister")?
Hi Gal, On 8/3/23 1:10 PM, Gal Pressman wrote: [...] > Our nightly regression testing picked up new memory leaks which were > bisected to this commit. > Unfortunately, I do not know the exact repro steps to trigger it, maybe > the attached kmemeleak logs can help? Is this on latest net-next? Do you have some more info on what the test is doing? Does it trigger on qdisc cleanup only? Also, is there a way to run the regression suite? Thanks, Daniel > unreferenced object 0xffff88811ce37b80 (size 224): > comm "kworker/14:1", pid 7451, jiffies 4295350041 (age 64.444s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 e0 69 29 81 88 ff ff 00 3a 39 0d 81 88 ff ff ..i).....:9..... > backtrace: > [<00000000a0f098fe>] __alloc_skb+0x1f4/0x2b0 > [<000000000dabee54>] alloc_skb_with_frags+0x7a/0x6c0 > [<00000000e681c78a>] sock_alloc_send_pskb+0x63f/0x7d0 > [<00000000a4774143>] mld_newpack.isra.0+0x1ad/0x800 > [<0000000060e32100>] add_grhead+0x271/0x320 > [<00000000040e7099>] add_grec+0xc8b/0x1120 > [<000000009853483c>] mld_ifc_work+0x387/0xae0 > [<0000000079d8299d>] process_one_work+0x86a/0x1430 > [<000000001968010b>] worker_thread+0x5b0/0xf00 > [<0000000090c285b0>] kthread+0x2dd/0x3b0 > [<000000001f322d79>] ret_from_fork+0x2d/0x70 > [<000000008ad6bd7b>] ret_from_fork_asm+0x11/0x20 > unreferenced object 0xffff888153058640 (size 224): > comm "softirq", pid 0, jiffies 4295350849 (age 61.212s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 e0 69 29 81 88 ff ff c0 32 39 0d 81 88 ff ff ..i).....29..... > backtrace: > [<00000000a0f098fe>] __alloc_skb+0x1f4/0x2b0 > [<00000000bb2ddb4c>] ndisc_alloc_skb+0x133/0x340 > [<0000000009614816>] ndisc_send_rs+0x1e0/0x4b0 > [<000000004bc1b8be>] addrconf_rs_timer+0x25a/0x720 > [<000000004d021706>] call_timer_fn+0x167/0x3d0 > [<0000000088aa76a3>] __run_timers.part.0+0x546/0x8b0 > [<0000000066f62ff3>] run_timer_softirq+0x6a/0x100 > [<000000003732ddfb>] __do_softirq+0x264/0x80c > unreferenced object 0xffff888155d0a500 (size 224): > comm "softirq", pid 0, jiffies 4295352832 (age 53.328s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 e0 69 29 81 88 ff ff c0 32 39 0d 81 88 ff ff ..i).....29..... > backtrace: > [<00000000a0f098fe>] __alloc_skb+0x1f4/0x2b0 > [<00000000bb2ddb4c>] ndisc_alloc_skb+0x133/0x340 > [<0000000009614816>] ndisc_send_rs+0x1e0/0x4b0 > [<000000004bc1b8be>] addrconf_rs_timer+0x25a/0x720 > [<000000004d021706>] call_timer_fn+0x167/0x3d0 > [<0000000088aa76a3>] __run_timers.part.0+0x546/0x8b0 > [<0000000066f62ff3>] run_timer_softirq+0x6a/0x100 > [<000000003732ddfb>] __do_softirq+0x264/0x80c > unreferenced object 0xffff88814e3f6040 (size 576): > comm "softirq", pid 0, jiffies 4295352832 (age 53.328s) > hex dump (first 32 bytes): > 00 00 33 33 00 00 00 02 e8 eb d3 98 21 bc 86 dd ..33........!... > 60 00 00 00 00 10 3a ff fe 80 00 00 00 00 00 00 `.....:......... > backtrace: > [<00000000525ad98b>] kmalloc_reserve+0x118/0x1f0 > [<000000008d146183>] __alloc_skb+0x105/0x2b0 > [<00000000bb2ddb4c>] ndisc_alloc_skb+0x133/0x340 > [<0000000009614816>] ndisc_send_rs+0x1e0/0x4b0 > [<000000004bc1b8be>] addrconf_rs_timer+0x25a/0x720 > [<000000004d021706>] call_timer_fn+0x167/0x3d0 > [<0000000088aa76a3>] __run_timers.part.0+0x546/0x8b0 > [<0000000066f62ff3>] run_timer_softirq+0x6a/0x100 > [<000000003732ddfb>] __do_softirq+0x264/0x80c > unreferenced object 0xffff88812acdebc0 (size 16): > comm "umount.nfs", pid 11626, jiffies 4295354796 (age 45.472s) > hex dump (first 16 bytes): > 73 65 72 76 65 72 2d 32 00 eb cd 2a 81 88 ff ff server-2...*.... > backtrace: > [<0000000010fb5130>] __kmalloc_node_track_caller+0x4c/0x170 > [<00000000b866a733>] kvasprintf+0xb0/0x130 > [<00000000b3564fca>] kasprintf+0xa6/0xd0 > [<00000000f01d6cb3>] nfs_sysfs_move_sb_to_server+0x49/0xd0 > [<000000009608708f>] nfs_kill_super+0x5f/0x90 > [<0000000090d4108b>] deactivate_locked_super+0x80/0x130 > [<000000000856aeb1>] cleanup_mnt+0x258/0x370 > [<0000000040582e39>] task_work_run+0x12c/0x210 > [<00000000378ea041>] exit_to_user_mode_prepare+0x1a0/0x1b0 > [<00000000025e63dd>] syscall_exit_to_user_mode+0x19/0x50 > [<00000000f34ad3ee>] do_syscall_64+0x4a/0x90 > [<000000009d3e2403>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 >
On Thu, Aug 03, 2023 at 02:10:51PM +0300, Gal Pressman wrote: > Our nightly regression testing picked up new memory leaks which were > bisected to this commit. > Unfortunately, I do not know the exact repro steps to trigger it, maybe > the attached kmemeleak logs can help? [...] > unreferenced object 0xffff88812acdebc0 (size 16): > comm "umount.nfs", pid 11626, jiffies 4295354796 (age 45.472s) > hex dump (first 16 bytes): > 73 65 72 76 65 72 2d 32 00 eb cd 2a 81 88 ff ff server-2...*.... > backtrace: > [<0000000010fb5130>] __kmalloc_node_track_caller+0x4c/0x170 > [<00000000b866a733>] kvasprintf+0xb0/0x130 > [<00000000b3564fca>] kasprintf+0xa6/0xd0 > [<00000000f01d6cb3>] nfs_sysfs_move_sb_to_server+0x49/0xd0 > [<000000009608708f>] nfs_kill_super+0x5f/0x90 > [<0000000090d4108b>] deactivate_locked_super+0x80/0x130 > [<000000000856aeb1>] cleanup_mnt+0x258/0x370 > [<0000000040582e39>] task_work_run+0x12c/0x210 > [<00000000378ea041>] exit_to_user_mode_prepare+0x1a0/0x1b0 > [<00000000025e63dd>] syscall_exit_to_user_mode+0x19/0x50 > [<00000000f34ad3ee>] do_syscall_64+0x4a/0x90 > [<000000009d3e2403>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 This one is caused by commit 1c7251187dc0 ("NFS: add superblock sysfs entries") and fixed by [1], so I'm not sure the bisection result is reliable. [1] https://lore.kernel.org/linux-nfs/6702796fee0365bf399800326bbe6c88e5f73f68.1689014440.git.bcodding@redhat.com/
On 04/08/2023 16:31, Ido Schimmel wrote: > On Thu, Aug 03, 2023 at 02:10:51PM +0300, Gal Pressman wrote: >> Our nightly regression testing picked up new memory leaks which were >> bisected to this commit. >> Unfortunately, I do not know the exact repro steps to trigger it, maybe >> the attached kmemeleak logs can help? > > [...] > >> unreferenced object 0xffff88812acdebc0 (size 16): >> comm "umount.nfs", pid 11626, jiffies 4295354796 (age 45.472s) >> hex dump (first 16 bytes): >> 73 65 72 76 65 72 2d 32 00 eb cd 2a 81 88 ff ff server-2...*.... >> backtrace: >> [<0000000010fb5130>] __kmalloc_node_track_caller+0x4c/0x170 >> [<00000000b866a733>] kvasprintf+0xb0/0x130 >> [<00000000b3564fca>] kasprintf+0xa6/0xd0 >> [<00000000f01d6cb3>] nfs_sysfs_move_sb_to_server+0x49/0xd0 >> [<000000009608708f>] nfs_kill_super+0x5f/0x90 >> [<0000000090d4108b>] deactivate_locked_super+0x80/0x130 >> [<000000000856aeb1>] cleanup_mnt+0x258/0x370 >> [<0000000040582e39>] task_work_run+0x12c/0x210 >> [<00000000378ea041>] exit_to_user_mode_prepare+0x1a0/0x1b0 >> [<00000000025e63dd>] syscall_exit_to_user_mode+0x19/0x50 >> [<00000000f34ad3ee>] do_syscall_64+0x4a/0x90 >> [<000000009d3e2403>] entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > This one is caused by commit 1c7251187dc0 ("NFS: add superblock sysfs > entries") and fixed by [1], so I'm not sure the bisection result is > reliable. > > [1] https://lore.kernel.org/linux-nfs/6702796fee0365bf399800326bbe6c88e5f73f68.1689014440.git.bcodding@redhat.com/ Thanks, maybe there is more than one issue lurking. We ran the bisect a few times, it always came back to this commit.
On 04/08/2023 2:48, Martin KaFai Lau wrote: > On 8/3/23 4:10 AM, Gal Pressman wrote: >> On 22/07/2023 2:33, Daniel Borkmann wrote: >>> On qdisc destruction, the ingress_destroy() needs to update the correct >>> entry, that is, tcx_entry_update must NULL the dev->tcx_ingress pointer. >>> Therefore, fix the typo. >>> >>> Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with >>> link support") >>> Reported-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com >>> Reported-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com >>> Reported-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com >>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> >> >> Hi Daniel, >> >> Our nightly regression testing picked up new memory leaks which were >> bisected to this commit. > > Thanks for the report. Can you help to check if it can be reproduced > with the latest net-next which has a tcx fix in commit 079082c60aff > ("tcx: Fix splat during dev unregister")? > I think the issue is resolved after this patch, but we're still verifying. Thanks!
On 04/08/2023 2:54, Daniel Borkmann wrote: > Hi Gal, > > On 8/3/23 1:10 PM, Gal Pressman wrote: > [...] >> Our nightly regression testing picked up new memory leaks which were >> bisected to this commit. >> Unfortunately, I do not know the exact repro steps to trigger it, maybe >> the attached kmemeleak logs can help? > > Is this on latest net-next? Do you have some more info on what the test > is doing? Does it trigger on qdisc cleanup only? Also, is there a way to > run the regression suite? I don't have more information ATM, I'll try to figure it out if the other fix doesn't resolve it.
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c index 04e886f6cee4..a463a63192c3 100644 --- a/net/sched/sch_ingress.c +++ b/net/sched/sch_ingress.c @@ -123,7 +123,7 @@ static void ingress_destroy(struct Qdisc *sch) if (entry) { tcx_miniq_set_active(entry, false); if (!tcx_entry_is_active(entry)) { - tcx_entry_update(dev, NULL, false); + tcx_entry_update(dev, NULL, true); tcx_entry_free(entry); } }
On qdisc destruction, the ingress_destroy() needs to update the correct entry, that is, tcx_entry_update must NULL the dev->tcx_ingress pointer. Therefore, fix the typo. Fixes: e420bed02507 ("bpf: Add fd-based tcx multi-prog infra with link support") Reported-by: syzbot+bdcf141f362ef83335cf@syzkaller.appspotmail.com Reported-by: syzbot+b202b7208664142954fa@syzkaller.appspotmail.com Reported-by: syzbot+14736e249bce46091c18@syzkaller.appspotmail.com Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> --- net/sched/sch_ingress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)