| Message ID | 20240408141039.30428-1-n.zhandarovich@fintech.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [net,v2] net/9p: fix uninit-value in p9_client_rpc() | expand |
On 4/8/24 07:10, Nikita Zhandarovich wrote: > Syzbot with the help of KMSAN reported the following error: > > BUG: KMSAN: uninit-value in trace_9p_client_res include/trace/events/9p.h:146 [inline] > BUG: KMSAN: uninit-value in p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 > trace_9p_client_res include/trace/events/9p.h:146 [inline] > p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 > p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 > v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 > v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 > legacy_get_tree+0x114/0x290 fs/fs_context.c:662 > vfs_get_tree+0xa7/0x570 fs/super.c:1797 > do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 > path_mount+0x742/0x1f20 fs/namespace.c:3679 > do_mount fs/namespace.c:3692 [inline] > __do_sys_mount fs/namespace.c:3898 [inline] > __se_sys_mount+0x725/0x810 fs/namespace.c:3875 > __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 > do_syscall_64+0xd5/0x1f0 > entry_SYSCALL_64_after_hwframe+0x6d/0x75 > > Uninit was created at: > __alloc_pages+0x9d6/0xe70 mm/page_alloc.c:4598 > __alloc_pages_node include/linux/gfp.h:238 [inline] > alloc_pages_node include/linux/gfp.h:261 [inline] > alloc_slab_page mm/slub.c:2175 [inline] > allocate_slab mm/slub.c:2338 [inline] > new_slab+0x2de/0x1400 mm/slub.c:2391 > ___slab_alloc+0x1184/0x33d0 mm/slub.c:3525 > __slab_alloc mm/slub.c:3610 [inline] > __slab_alloc_node mm/slub.c:3663 [inline] > slab_alloc_node mm/slub.c:3835 [inline] > kmem_cache_alloc+0x6d3/0xbe0 mm/slub.c:3852 > p9_tag_alloc net/9p/client.c:278 [inline] > p9_client_prepare_req+0x20a/0x1770 net/9p/client.c:641 > p9_client_rpc+0x27e/0x1340 net/9p/client.c:688 > p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 > v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 > v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 > legacy_get_tree+0x114/0x290 fs/fs_context.c:662 > vfs_get_tree+0xa7/0x570 fs/super.c:1797 > do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 > path_mount+0x742/0x1f20 fs/namespace.c:3679 > do_mount fs/namespace.c:3692 [inline] > __do_sys_mount fs/namespace.c:3898 [inline] > __se_sys_mount+0x725/0x810 fs/namespace.c:3875 > __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 > do_syscall_64+0xd5/0x1f0 > entry_SYSCALL_64_after_hwframe+0x6d/0x75 > > If p9_check_errors() fails early in p9_client_rpc(), req->rc.tag > will not be properly initialized. However, trace_9p_client_res() > ends up trying to print it out anyway before p9_client_rpc() > finishes. > > Fix this issue by assigning default values to p9_fcall fields > such as 'tag' and (just in case KMSAN unearths something new) 'id' > during the tag allocation stage. > > Reported-and-tested-by: syzbot+ff14db38f56329ef68df@syzkaller.appspotmail.com > Fixes: 348b59012e5c ("net/9p: Convert net/9p protocol dumps to tracepoints") > Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> > --- > v2: change fc->tag init value from 0 to P9_NOTAG per Dominique > Martinet's <asmadeus@codewreck.org> helpful suggestion. > Link: https://lore.kernel.org/all/ZhNVMivKCCq6wir0@codewreck.org/ > > net/9p/client.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/9p/client.c b/net/9p/client.c > index f7e90b4769bb..b05f73c291b4 100644 > --- a/net/9p/client.c > +++ b/net/9p/client.c > @@ -235,6 +235,8 @@ static int p9_fcall_init(struct p9_client *c, struct p9_fcall *fc, > if (!fc->sdata) > return -ENOMEM; > fc->capacity = alloc_msize; > + fc->id = 0; > + fc->tag = P9_NOTAG; > return 0; > } > Hi Dominique, Gentle ping on this issue as it is still open. Thanks in advance :) Regards, Nikita
On Monday, April 8, 2024 4:10:39 PM CEST Nikita Zhandarovich wrote: > Syzbot with the help of KMSAN reported the following error: > > BUG: KMSAN: uninit-value in trace_9p_client_res include/trace/events/9p.h:146 [inline] > BUG: KMSAN: uninit-value in p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 > trace_9p_client_res include/trace/events/9p.h:146 [inline] > p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 > p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 > v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 > v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 > legacy_get_tree+0x114/0x290 fs/fs_context.c:662 > vfs_get_tree+0xa7/0x570 fs/super.c:1797 > do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 > path_mount+0x742/0x1f20 fs/namespace.c:3679 > do_mount fs/namespace.c:3692 [inline] > __do_sys_mount fs/namespace.c:3898 [inline] > __se_sys_mount+0x725/0x810 fs/namespace.c:3875 > __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 > do_syscall_64+0xd5/0x1f0 > entry_SYSCALL_64_after_hwframe+0x6d/0x75 > > Uninit was created at: > __alloc_pages+0x9d6/0xe70 mm/page_alloc.c:4598 > __alloc_pages_node include/linux/gfp.h:238 [inline] > alloc_pages_node include/linux/gfp.h:261 [inline] > alloc_slab_page mm/slub.c:2175 [inline] > allocate_slab mm/slub.c:2338 [inline] > new_slab+0x2de/0x1400 mm/slub.c:2391 > ___slab_alloc+0x1184/0x33d0 mm/slub.c:3525 > __slab_alloc mm/slub.c:3610 [inline] > __slab_alloc_node mm/slub.c:3663 [inline] > slab_alloc_node mm/slub.c:3835 [inline] > kmem_cache_alloc+0x6d3/0xbe0 mm/slub.c:3852 > p9_tag_alloc net/9p/client.c:278 [inline] > p9_client_prepare_req+0x20a/0x1770 net/9p/client.c:641 > p9_client_rpc+0x27e/0x1340 net/9p/client.c:688 > p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 > v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 > v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 > legacy_get_tree+0x114/0x290 fs/fs_context.c:662 > vfs_get_tree+0xa7/0x570 fs/super.c:1797 > do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 > path_mount+0x742/0x1f20 fs/namespace.c:3679 > do_mount fs/namespace.c:3692 [inline] > __do_sys_mount fs/namespace.c:3898 [inline] > __se_sys_mount+0x725/0x810 fs/namespace.c:3875 > __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 > do_syscall_64+0xd5/0x1f0 > entry_SYSCALL_64_after_hwframe+0x6d/0x75 > > If p9_check_errors() fails early in p9_client_rpc(), req->rc.tag > will not be properly initialized. However, trace_9p_client_res() > ends up trying to print it out anyway before p9_client_rpc() > finishes. > > Fix this issue by assigning default values to p9_fcall fields > such as 'tag' and (just in case KMSAN unearths something new) 'id' > during the tag allocation stage. > > Reported-and-tested-by: syzbot+ff14db38f56329ef68df@syzkaller.appspotmail.com > Fixes: 348b59012e5c ("net/9p: Convert net/9p protocol dumps to tracepoints") > Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> > --- > v2: change fc->tag init value from 0 to P9_NOTAG per Dominique > Martinet's <asmadeus@codewreck.org> helpful suggestion. > Link: https://lore.kernel.org/all/ZhNVMivKCCq6wir0@codewreck.org/ > > net/9p/client.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com> > diff --git a/net/9p/client.c b/net/9p/client.c > index f7e90b4769bb..b05f73c291b4 100644 > --- a/net/9p/client.c > +++ b/net/9p/client.c > @@ -235,6 +235,8 @@ static int p9_fcall_init(struct p9_client *c, struct p9_fcall *fc, > if (!fc->sdata) > return -ENOMEM; > fc->capacity = alloc_msize; > + fc->id = 0; > + fc->tag = P9_NOTAG; > return 0; > } > >
Nikita Zhandarovich wrote on Wed, May 15, 2024 at 08:18:19AM -0700:
> Gentle ping on this issue as it is still open. Thanks in advance :)
Thanks for the ping and sorry for the inactivity -- your patch came in
the middle of a rough patch (ha, ha..) with a couple of regressions that
drained me and we didn't get any other outside patch this cycle, so I
didn't prepare a pull request for Linus.
(In theory, it should have made it in one of Eric's pull for 6.9-rc5/6,
but I guess it slipped through his mailer as well)
I'll queue the patch to -next now and send it next week if nothing else
pops up.
diff --git a/net/9p/client.c b/net/9p/client.c index f7e90b4769bb..b05f73c291b4 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -235,6 +235,8 @@ static int p9_fcall_init(struct p9_client *c, struct p9_fcall *fc, if (!fc->sdata) return -ENOMEM; fc->capacity = alloc_msize; + fc->id = 0; + fc->tag = P9_NOTAG; return 0; }
Syzbot with the help of KMSAN reported the following error: BUG: KMSAN: uninit-value in trace_9p_client_res include/trace/events/9p.h:146 [inline] BUG: KMSAN: uninit-value in p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 trace_9p_client_res include/trace/events/9p.h:146 [inline] p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 legacy_get_tree+0x114/0x290 fs/fs_context.c:662 vfs_get_tree+0xa7/0x570 fs/super.c:1797 do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 path_mount+0x742/0x1f20 fs/namespace.c:3679 do_mount fs/namespace.c:3692 [inline] __do_sys_mount fs/namespace.c:3898 [inline] __se_sys_mount+0x725/0x810 fs/namespace.c:3875 __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 do_syscall_64+0xd5/0x1f0 entry_SYSCALL_64_after_hwframe+0x6d/0x75 Uninit was created at: __alloc_pages+0x9d6/0xe70 mm/page_alloc.c:4598 __alloc_pages_node include/linux/gfp.h:238 [inline] alloc_pages_node include/linux/gfp.h:261 [inline] alloc_slab_page mm/slub.c:2175 [inline] allocate_slab mm/slub.c:2338 [inline] new_slab+0x2de/0x1400 mm/slub.c:2391 ___slab_alloc+0x1184/0x33d0 mm/slub.c:3525 __slab_alloc mm/slub.c:3610 [inline] __slab_alloc_node mm/slub.c:3663 [inline] slab_alloc_node mm/slub.c:3835 [inline] kmem_cache_alloc+0x6d3/0xbe0 mm/slub.c:3852 p9_tag_alloc net/9p/client.c:278 [inline] p9_client_prepare_req+0x20a/0x1770 net/9p/client.c:641 p9_client_rpc+0x27e/0x1340 net/9p/client.c:688 p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 legacy_get_tree+0x114/0x290 fs/fs_context.c:662 vfs_get_tree+0xa7/0x570 fs/super.c:1797 do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 path_mount+0x742/0x1f20 fs/namespace.c:3679 do_mount fs/namespace.c:3692 [inline] __do_sys_mount fs/namespace.c:3898 [inline] __se_sys_mount+0x725/0x810 fs/namespace.c:3875 __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 do_syscall_64+0xd5/0x1f0 entry_SYSCALL_64_after_hwframe+0x6d/0x75 If p9_check_errors() fails early in p9_client_rpc(), req->rc.tag will not be properly initialized. However, trace_9p_client_res() ends up trying to print it out anyway before p9_client_rpc() finishes. Fix this issue by assigning default values to p9_fcall fields such as 'tag' and (just in case KMSAN unearths something new) 'id' during the tag allocation stage. Reported-and-tested-by: syzbot+ff14db38f56329ef68df@syzkaller.appspotmail.com Fixes: 348b59012e5c ("net/9p: Convert net/9p protocol dumps to tracepoints") Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> --- v2: change fc->tag init value from 0 to P9_NOTAG per Dominique Martinet's <asmadeus@codewreck.org> helpful suggestion. Link: https://lore.kernel.org/all/ZhNVMivKCCq6wir0@codewreck.org/ net/9p/client.c | 2 ++ 1 file changed, 2 insertions(+)