| Message ID | 20221027124528.2487025-1-zengheng4@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] cifs: fix use-after-free caused by invalid pointer `hostname` | expand |
merged into cifs-2.6 for-next thx On Thu, Oct 27, 2022 at 7:49 AM Zeng Heng <zengheng4@huawei.com> wrote: > > `hostname` needs to be set as null-pointer after free in > `cifs_put_tcp_session` function, or when `cifsd` thread attempts > to resolve hostname and reconnect the host, the thread would deref > the invalid pointer. > > Here is one of practical backtrace examples as reference: > > Task 477 > --------------------------- > do_mount > path_mount > do_new_mount > vfs_get_tree > smb3_get_tree > smb3_get_tree_common > cifs_smb3_do_mount > cifs_mount > mount_put_conns > cifs_put_tcp_session > --> kfree(server->hostname) > > cifsd > --------------------------- > kthread > cifs_demultiplex_thread > cifs_reconnect > reconn_set_ipaddr_from_hostname > --> if (!server->hostname) > --> if (server->hostname[0] == '\0') // !! UAF fault here > > CIFS: VFS: cifs_mount failed w/return code = -112 > mount error(112): Host is down > BUG: KASAN: use-after-free in reconn_set_ipaddr_from_hostname+0x2ba/0x310 > Read of size 1 at addr ffff888108f35380 by task cifsd/480 > CPU: 2 PID: 480 Comm: cifsd Not tainted 6.1.0-rc2-00106-gf705792f89dd-dirty #25 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > Call Trace: > <TASK> > dump_stack_lvl+0x68/0x85 > print_report+0x16c/0x4a3 > kasan_report+0x95/0x190 > reconn_set_ipaddr_from_hostname+0x2ba/0x310 > __cifs_reconnect.part.0+0x241/0x800 > cifs_reconnect+0x65f/0xb60 > cifs_demultiplex_thread+0x1570/0x2570 > kthread+0x2c5/0x380 > ret_from_fork+0x22/0x30 > </TASK> > Allocated by task 477: > kasan_save_stack+0x1e/0x40 > kasan_set_track+0x21/0x30 > __kasan_kmalloc+0x7e/0x90 > __kmalloc_node_track_caller+0x52/0x1b0 > kstrdup+0x3b/0x70 > cifs_get_tcp_session+0xbc/0x19b0 > mount_get_conns+0xa9/0x10c0 > cifs_mount+0xdf/0x1970 > cifs_smb3_do_mount+0x295/0x1660 > smb3_get_tree+0x352/0x5e0 > vfs_get_tree+0x8e/0x2e0 > path_mount+0xf8c/0x1990 > do_mount+0xee/0x110 > __x64_sys_mount+0x14b/0x1f0 > do_syscall_64+0x3b/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > Freed by task 477: > kasan_save_stack+0x1e/0x40 > kasan_set_track+0x21/0x30 > kasan_save_free_info+0x2a/0x50 > __kasan_slab_free+0x10a/0x190 > __kmem_cache_free+0xca/0x3f0 > cifs_put_tcp_session+0x30c/0x450 > cifs_mount+0xf95/0x1970 > cifs_smb3_do_mount+0x295/0x1660 > smb3_get_tree+0x352/0x5e0 > vfs_get_tree+0x8e/0x2e0 > path_mount+0xf8c/0x1990 > do_mount+0xee/0x110 > __x64_sys_mount+0x14b/0x1f0 > do_syscall_64+0x3b/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > The buggy address belongs to the object at ffff888108f35380 > which belongs to the cache kmalloc-16 of size 16 > The buggy address is located 0 bytes inside of > 16-byte region [ffff888108f35380, ffff888108f35390) > The buggy address belongs to the physical page: > page:00000000333f8e58 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888108f350e0 pfn:0x108f35 > flags: 0x200000000000200(slab|node=0|zone=2) > raw: 0200000000000200 0000000000000000 dead000000000122 ffff8881000423c0 > raw: ffff888108f350e0 000000008080007a 00000001ffffffff 0000000000000000 > page dumped because: kasan: bad access detected > Memory state around the buggy address: > ffff888108f35280: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > ffff888108f35300: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > >ffff888108f35380: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > ^ > ffff888108f35400: fa fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc > ffff888108f35480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > Fixes: 7be3248f3139 ("cifs: To match file servers, make sure the server hostname matches") > Signed-off-by: Zeng Heng <zengheng4@huawei.com> > Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > --- > changes in v4: > - correct fix tag > - add reviewed-by > --- > fs/cifs/connect.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index ffb291579bb9..1cc47dd3b4d6 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -1584,6 +1584,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) > server->session_key.response = NULL; > server->session_key.len = 0; > kfree(server->hostname); > + server->hostname = NULL; > > task = xchg(&server->tsk, NULL); > if (task) > -- > 2.25.1 >
On Thu, Oct 27, 2022 at 6:19 PM Zeng Heng <zengheng4@huawei.com> wrote: > > `hostname` needs to be set as null-pointer after free in > `cifs_put_tcp_session` function, or when `cifsd` thread attempts > to resolve hostname and reconnect the host, the thread would deref > the invalid pointer. > > Here is one of practical backtrace examples as reference: > > Task 477 > --------------------------- > do_mount > path_mount > do_new_mount > vfs_get_tree > smb3_get_tree > smb3_get_tree_common > cifs_smb3_do_mount > cifs_mount > mount_put_conns > cifs_put_tcp_session > --> kfree(server->hostname) > > cifsd > --------------------------- > kthread > cifs_demultiplex_thread > cifs_reconnect > reconn_set_ipaddr_from_hostname > --> if (!server->hostname) > --> if (server->hostname[0] == '\0') // !! UAF fault here > > CIFS: VFS: cifs_mount failed w/return code = -112 > mount error(112): Host is down > BUG: KASAN: use-after-free in reconn_set_ipaddr_from_hostname+0x2ba/0x310 > Read of size 1 at addr ffff888108f35380 by task cifsd/480 > CPU: 2 PID: 480 Comm: cifsd Not tainted 6.1.0-rc2-00106-gf705792f89dd-dirty #25 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > Call Trace: > <TASK> > dump_stack_lvl+0x68/0x85 > print_report+0x16c/0x4a3 > kasan_report+0x95/0x190 > reconn_set_ipaddr_from_hostname+0x2ba/0x310 > __cifs_reconnect.part.0+0x241/0x800 > cifs_reconnect+0x65f/0xb60 > cifs_demultiplex_thread+0x1570/0x2570 > kthread+0x2c5/0x380 > ret_from_fork+0x22/0x30 > </TASK> > Allocated by task 477: > kasan_save_stack+0x1e/0x40 > kasan_set_track+0x21/0x30 > __kasan_kmalloc+0x7e/0x90 > __kmalloc_node_track_caller+0x52/0x1b0 > kstrdup+0x3b/0x70 > cifs_get_tcp_session+0xbc/0x19b0 > mount_get_conns+0xa9/0x10c0 > cifs_mount+0xdf/0x1970 > cifs_smb3_do_mount+0x295/0x1660 > smb3_get_tree+0x352/0x5e0 > vfs_get_tree+0x8e/0x2e0 > path_mount+0xf8c/0x1990 > do_mount+0xee/0x110 > __x64_sys_mount+0x14b/0x1f0 > do_syscall_64+0x3b/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > Freed by task 477: > kasan_save_stack+0x1e/0x40 > kasan_set_track+0x21/0x30 > kasan_save_free_info+0x2a/0x50 > __kasan_slab_free+0x10a/0x190 > __kmem_cache_free+0xca/0x3f0 > cifs_put_tcp_session+0x30c/0x450 > cifs_mount+0xf95/0x1970 > cifs_smb3_do_mount+0x295/0x1660 > smb3_get_tree+0x352/0x5e0 > vfs_get_tree+0x8e/0x2e0 > path_mount+0xf8c/0x1990 > do_mount+0xee/0x110 > __x64_sys_mount+0x14b/0x1f0 > do_syscall_64+0x3b/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > The buggy address belongs to the object at ffff888108f35380 > which belongs to the cache kmalloc-16 of size 16 > The buggy address is located 0 bytes inside of > 16-byte region [ffff888108f35380, ffff888108f35390) > The buggy address belongs to the physical page: > page:00000000333f8e58 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888108f350e0 pfn:0x108f35 > flags: 0x200000000000200(slab|node=0|zone=2) > raw: 0200000000000200 0000000000000000 dead000000000122 ffff8881000423c0 > raw: ffff888108f350e0 000000008080007a 00000001ffffffff 0000000000000000 > page dumped because: kasan: bad access detected > Memory state around the buggy address: > ffff888108f35280: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > ffff888108f35300: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > >ffff888108f35380: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > ^ > ffff888108f35400: fa fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc > ffff888108f35480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > Fixes: 7be3248f3139 ("cifs: To match file servers, make sure the server hostname matches") > Signed-off-by: Zeng Heng <zengheng4@huawei.com> > Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > --- > changes in v4: > - correct fix tag > - add reviewed-by > --- > fs/cifs/connect.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index ffb291579bb9..1cc47dd3b4d6 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -1584,6 +1584,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) > server->session_key.response = NULL; > server->session_key.len = 0; > kfree(server->hostname); > + server->hostname = NULL; > > task = xchg(&server->tsk, NULL); > if (task) > -- > 2.25.1 > Good catch. But I think there can be a better fix. How about moving the lines that follow i.e. cifsd thread kill to before setting tcpStatus? That way, we don't leave scope for things to race.
On Fri, Oct 28, 2022 at 11:29 AM Steve French <smfrench@gmail.com> wrote: > > merged into cifs-2.6 for-next > > thx > > On Thu, Oct 27, 2022 at 7:49 AM Zeng Heng <zengheng4@huawei.com> wrote: > > > > `hostname` needs to be set as null-pointer after free in > > `cifs_put_tcp_session` function, or when `cifsd` thread attempts > > to resolve hostname and reconnect the host, the thread would deref > > the invalid pointer. > > > > Here is one of practical backtrace examples as reference: > > > > Task 477 > > --------------------------- > > do_mount > > path_mount > > do_new_mount > > vfs_get_tree > > smb3_get_tree > > smb3_get_tree_common > > cifs_smb3_do_mount > > cifs_mount > > mount_put_conns > > cifs_put_tcp_session > > --> kfree(server->hostname) > > > > cifsd > > --------------------------- > > kthread > > cifs_demultiplex_thread > > cifs_reconnect > > reconn_set_ipaddr_from_hostname > > --> if (!server->hostname) > > --> if (server->hostname[0] == '\0') // !! UAF fault here > > > > CIFS: VFS: cifs_mount failed w/return code = -112 > > mount error(112): Host is down > > BUG: KASAN: use-after-free in reconn_set_ipaddr_from_hostname+0x2ba/0x310 > > Read of size 1 at addr ffff888108f35380 by task cifsd/480 > > CPU: 2 PID: 480 Comm: cifsd Not tainted 6.1.0-rc2-00106-gf705792f89dd-dirty #25 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > > Call Trace: > > <TASK> > > dump_stack_lvl+0x68/0x85 > > print_report+0x16c/0x4a3 > > kasan_report+0x95/0x190 > > reconn_set_ipaddr_from_hostname+0x2ba/0x310 > > __cifs_reconnect.part.0+0x241/0x800 > > cifs_reconnect+0x65f/0xb60 > > cifs_demultiplex_thread+0x1570/0x2570 > > kthread+0x2c5/0x380 > > ret_from_fork+0x22/0x30 > > </TASK> > > Allocated by task 477: > > kasan_save_stack+0x1e/0x40 > > kasan_set_track+0x21/0x30 > > __kasan_kmalloc+0x7e/0x90 > > __kmalloc_node_track_caller+0x52/0x1b0 > > kstrdup+0x3b/0x70 > > cifs_get_tcp_session+0xbc/0x19b0 > > mount_get_conns+0xa9/0x10c0 > > cifs_mount+0xdf/0x1970 > > cifs_smb3_do_mount+0x295/0x1660 > > smb3_get_tree+0x352/0x5e0 > > vfs_get_tree+0x8e/0x2e0 > > path_mount+0xf8c/0x1990 > > do_mount+0xee/0x110 > > __x64_sys_mount+0x14b/0x1f0 > > do_syscall_64+0x3b/0x90 > > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > Freed by task 477: > > kasan_save_stack+0x1e/0x40 > > kasan_set_track+0x21/0x30 > > kasan_save_free_info+0x2a/0x50 > > __kasan_slab_free+0x10a/0x190 > > __kmem_cache_free+0xca/0x3f0 > > cifs_put_tcp_session+0x30c/0x450 > > cifs_mount+0xf95/0x1970 > > cifs_smb3_do_mount+0x295/0x1660 > > smb3_get_tree+0x352/0x5e0 > > vfs_get_tree+0x8e/0x2e0 > > path_mount+0xf8c/0x1990 > > do_mount+0xee/0x110 > > __x64_sys_mount+0x14b/0x1f0 > > do_syscall_64+0x3b/0x90 > > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > The buggy address belongs to the object at ffff888108f35380 > > which belongs to the cache kmalloc-16 of size 16 > > The buggy address is located 0 bytes inside of > > 16-byte region [ffff888108f35380, ffff888108f35390) > > The buggy address belongs to the physical page: > > page:00000000333f8e58 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888108f350e0 pfn:0x108f35 > > flags: 0x200000000000200(slab|node=0|zone=2) > > raw: 0200000000000200 0000000000000000 dead000000000122 ffff8881000423c0 > > raw: ffff888108f350e0 000000008080007a 00000001ffffffff 0000000000000000 > > page dumped because: kasan: bad access detected > > Memory state around the buggy address: > > ffff888108f35280: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > > ffff888108f35300: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > > >ffff888108f35380: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > > ^ > > ffff888108f35400: fa fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > ffff888108f35480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > > > Fixes: 7be3248f3139 ("cifs: To match file servers, make sure the server hostname matches") > > Signed-off-by: Zeng Heng <zengheng4@huawei.com> > > Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > > --- > > changes in v4: > > - correct fix tag > > - add reviewed-by > > --- > > fs/cifs/connect.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > > index ffb291579bb9..1cc47dd3b4d6 100644 > > --- a/fs/cifs/connect.c > > +++ b/fs/cifs/connect.c > > @@ -1584,6 +1584,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) > > server->session_key.response = NULL; > > server->session_key.len = 0; > > kfree(server->hostname); > > + server->hostname = NULL; > > > > task = xchg(&server->tsk, NULL); > > if (task) > > -- > > 2.25.1 > > > > > -- > Thanks, > > Steve What I mean is something like this: https://github.com/sprasad-microsoft/smb3-kernel-client/commit/07abfbeb01d3cb8d55d76c1937bd4cde46063e5d.patch
Make sure `cifsd` terminated to avoid race condition, it has to call function like kthread_stop. Then the whole `server` struct would be released by `cifsd` and another UAF appears. On 2022/10/28 13:41, Shyam Prasad N wrote: > On Thu, Oct 27, 2022 at 6:19 PM Zeng Heng <zengheng4@huawei.com> wrote: >> `hostname` needs to be set as null-pointer after free in >> `cifs_put_tcp_session` function, or when `cifsd` thread attempts >> to resolve hostname and reconnect the host, the thread would deref >> the invalid pointer. >> >> Here is one of practical backtrace examples as reference: >> >> Task 477 >> --------------------------- >> do_mount >> path_mount >> do_new_mount >> vfs_get_tree >> smb3_get_tree >> smb3_get_tree_common >> cifs_smb3_do_mount >> cifs_mount >> mount_put_conns >> cifs_put_tcp_session >> --> kfree(server->hostname) >> >> cifsd >> --------------------------- >> kthread >> cifs_demultiplex_thread >> cifs_reconnect >> reconn_set_ipaddr_from_hostname >> --> if (!server->hostname) >> --> if (server->hostname[0] == '\0') // !! UAF fault here >> >> CIFS: VFS: cifs_mount failed w/return code = -112 >> mount error(112): Host is down >> BUG: KASAN: use-after-free in reconn_set_ipaddr_from_hostname+0x2ba/0x310 >> Read of size 1 at addr ffff888108f35380 by task cifsd/480 >> CPU: 2 PID: 480 Comm: cifsd Not tainted 6.1.0-rc2-00106-gf705792f89dd-dirty #25 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 >> Call Trace: >> <TASK> >> dump_stack_lvl+0x68/0x85 >> print_report+0x16c/0x4a3 >> kasan_report+0x95/0x190 >> reconn_set_ipaddr_from_hostname+0x2ba/0x310 >> __cifs_reconnect.part.0+0x241/0x800 >> cifs_reconnect+0x65f/0xb60 >> cifs_demultiplex_thread+0x1570/0x2570 >> kthread+0x2c5/0x380 >> ret_from_fork+0x22/0x30 >> </TASK> >> Allocated by task 477: >> kasan_save_stack+0x1e/0x40 >> kasan_set_track+0x21/0x30 >> __kasan_kmalloc+0x7e/0x90 >> __kmalloc_node_track_caller+0x52/0x1b0 >> kstrdup+0x3b/0x70 >> cifs_get_tcp_session+0xbc/0x19b0 >> mount_get_conns+0xa9/0x10c0 >> cifs_mount+0xdf/0x1970 >> cifs_smb3_do_mount+0x295/0x1660 >> smb3_get_tree+0x352/0x5e0 >> vfs_get_tree+0x8e/0x2e0 >> path_mount+0xf8c/0x1990 >> do_mount+0xee/0x110 >> __x64_sys_mount+0x14b/0x1f0 >> do_syscall_64+0x3b/0x90 >> entry_SYSCALL_64_after_hwframe+0x63/0xcd >> Freed by task 477: >> kasan_save_stack+0x1e/0x40 >> kasan_set_track+0x21/0x30 >> kasan_save_free_info+0x2a/0x50 >> __kasan_slab_free+0x10a/0x190 >> __kmem_cache_free+0xca/0x3f0 >> cifs_put_tcp_session+0x30c/0x450 >> cifs_mount+0xf95/0x1970 >> cifs_smb3_do_mount+0x295/0x1660 >> smb3_get_tree+0x352/0x5e0 >> vfs_get_tree+0x8e/0x2e0 >> path_mount+0xf8c/0x1990 >> do_mount+0xee/0x110 >> __x64_sys_mount+0x14b/0x1f0 >> do_syscall_64+0x3b/0x90 >> entry_SYSCALL_64_after_hwframe+0x63/0xcd >> The buggy address belongs to the object at ffff888108f35380 >> which belongs to the cache kmalloc-16 of size 16 >> The buggy address is located 0 bytes inside of >> 16-byte region [ffff888108f35380, ffff888108f35390) >> The buggy address belongs to the physical page: >> page:00000000333f8e58 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888108f350e0 pfn:0x108f35 >> flags: 0x200000000000200(slab|node=0|zone=2) >> raw: 0200000000000200 0000000000000000 dead000000000122 ffff8881000423c0 >> raw: ffff888108f350e0 000000008080007a 00000001ffffffff 0000000000000000 >> page dumped because: kasan: bad access detected >> Memory state around the buggy address: >> ffff888108f35280: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc >> ffff888108f35300: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc >>> ffff888108f35380: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc >> ^ >> ffff888108f35400: fa fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc >> ffff888108f35480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc >> >> Fixes: 7be3248f3139 ("cifs: To match file servers, make sure the server hostname matches") >> Signed-off-by: Zeng Heng <zengheng4@huawei.com> >> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> >> --- >> changes in v4: >> - correct fix tag >> - add reviewed-by >> --- >> fs/cifs/connect.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c >> index ffb291579bb9..1cc47dd3b4d6 100644 >> --- a/fs/cifs/connect.c >> +++ b/fs/cifs/connect.c >> @@ -1584,6 +1584,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) >> server->session_key.response = NULL; >> server->session_key.len = 0; >> kfree(server->hostname); >> + server->hostname = NULL; >> >> task = xchg(&server->tsk, NULL); >> if (task) >> -- >> 2.25.1 >> > Good catch. But I think there can be a better fix. > How about moving the lines that follow i.e. cifsd thread kill to > before setting tcpStatus? That way, we don't leave scope for things to > race. >
On Sat, Oct 29, 2022 at 8:25 AM Zeng Heng <zengheng4@huawei.com> wrote: > > Make sure `cifsd` terminated to avoid race condition, it has to call > function like kthread_stop. > > Then the whole `server` struct would be released by `cifsd` and another > UAF appears. > > > On 2022/10/28 13:41, Shyam Prasad N wrote: > > On Thu, Oct 27, 2022 at 6:19 PM Zeng Heng <zengheng4@huawei.com> wrote: > >> `hostname` needs to be set as null-pointer after free in > >> `cifs_put_tcp_session` function, or when `cifsd` thread attempts > >> to resolve hostname and reconnect the host, the thread would deref > >> the invalid pointer. > >> > >> Here is one of practical backtrace examples as reference: > >> > >> Task 477 > >> --------------------------- > >> do_mount > >> path_mount > >> do_new_mount > >> vfs_get_tree > >> smb3_get_tree > >> smb3_get_tree_common > >> cifs_smb3_do_mount > >> cifs_mount > >> mount_put_conns > >> cifs_put_tcp_session > >> --> kfree(server->hostname) > >> > >> cifsd > >> --------------------------- > >> kthread > >> cifs_demultiplex_thread > >> cifs_reconnect > >> reconn_set_ipaddr_from_hostname > >> --> if (!server->hostname) > >> --> if (server->hostname[0] == '\0') // !! UAF fault here > >> > >> CIFS: VFS: cifs_mount failed w/return code = -112 > >> mount error(112): Host is down > >> BUG: KASAN: use-after-free in reconn_set_ipaddr_from_hostname+0x2ba/0x310 > >> Read of size 1 at addr ffff888108f35380 by task cifsd/480 > >> CPU: 2 PID: 480 Comm: cifsd Not tainted 6.1.0-rc2-00106-gf705792f89dd-dirty #25 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > >> Call Trace: > >> <TASK> > >> dump_stack_lvl+0x68/0x85 > >> print_report+0x16c/0x4a3 > >> kasan_report+0x95/0x190 > >> reconn_set_ipaddr_from_hostname+0x2ba/0x310 > >> __cifs_reconnect.part.0+0x241/0x800 > >> cifs_reconnect+0x65f/0xb60 > >> cifs_demultiplex_thread+0x1570/0x2570 > >> kthread+0x2c5/0x380 > >> ret_from_fork+0x22/0x30 > >> </TASK> > >> Allocated by task 477: > >> kasan_save_stack+0x1e/0x40 > >> kasan_set_track+0x21/0x30 > >> __kasan_kmalloc+0x7e/0x90 > >> __kmalloc_node_track_caller+0x52/0x1b0 > >> kstrdup+0x3b/0x70 > >> cifs_get_tcp_session+0xbc/0x19b0 > >> mount_get_conns+0xa9/0x10c0 > >> cifs_mount+0xdf/0x1970 > >> cifs_smb3_do_mount+0x295/0x1660 > >> smb3_get_tree+0x352/0x5e0 > >> vfs_get_tree+0x8e/0x2e0 > >> path_mount+0xf8c/0x1990 > >> do_mount+0xee/0x110 > >> __x64_sys_mount+0x14b/0x1f0 > >> do_syscall_64+0x3b/0x90 > >> entry_SYSCALL_64_after_hwframe+0x63/0xcd > >> Freed by task 477: > >> kasan_save_stack+0x1e/0x40 > >> kasan_set_track+0x21/0x30 > >> kasan_save_free_info+0x2a/0x50 > >> __kasan_slab_free+0x10a/0x190 > >> __kmem_cache_free+0xca/0x3f0 > >> cifs_put_tcp_session+0x30c/0x450 > >> cifs_mount+0xf95/0x1970 > >> cifs_smb3_do_mount+0x295/0x1660 > >> smb3_get_tree+0x352/0x5e0 > >> vfs_get_tree+0x8e/0x2e0 > >> path_mount+0xf8c/0x1990 > >> do_mount+0xee/0x110 > >> __x64_sys_mount+0x14b/0x1f0 > >> do_syscall_64+0x3b/0x90 > >> entry_SYSCALL_64_after_hwframe+0x63/0xcd > >> The buggy address belongs to the object at ffff888108f35380 > >> which belongs to the cache kmalloc-16 of size 16 > >> The buggy address is located 0 bytes inside of > >> 16-byte region [ffff888108f35380, ffff888108f35390) > >> The buggy address belongs to the physical page: > >> page:00000000333f8e58 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888108f350e0 pfn:0x108f35 > >> flags: 0x200000000000200(slab|node=0|zone=2) > >> raw: 0200000000000200 0000000000000000 dead000000000122 ffff8881000423c0 > >> raw: ffff888108f350e0 000000008080007a 00000001ffffffff 0000000000000000 > >> page dumped because: kasan: bad access detected > >> Memory state around the buggy address: > >> ffff888108f35280: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > >> ffff888108f35300: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > >>> ffff888108f35380: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc > >> ^ > >> ffff888108f35400: fa fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc > >> ffff888108f35480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > >> > >> Fixes: 7be3248f3139 ("cifs: To match file servers, make sure the server hostname matches") > >> Signed-off-by: Zeng Heng <zengheng4@huawei.com> > >> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > >> --- > >> changes in v4: > >> - correct fix tag > >> - add reviewed-by > >> --- > >> fs/cifs/connect.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > >> index ffb291579bb9..1cc47dd3b4d6 100644 > >> --- a/fs/cifs/connect.c > >> +++ b/fs/cifs/connect.c > >> @@ -1584,6 +1584,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) > >> server->session_key.response = NULL; > >> server->session_key.len = 0; > >> kfree(server->hostname); > >> + server->hostname = NULL; > >> > >> task = xchg(&server->tsk, NULL); > >> if (task) > >> -- > >> 2.25.1 > >> > > Good catch. But I think there can be a better fix. > > How about moving the lines that follow i.e. cifsd thread kill to > > before setting tcpStatus? That way, we don't leave scope for things to > > race. > > Good point. We need to ensure that.
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index ffb291579bb9..1cc47dd3b4d6 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1584,6 +1584,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) server->session_key.response = NULL; server->session_key.len = 0; kfree(server->hostname); + server->hostname = NULL; task = xchg(&server->tsk, NULL); if (task)
`hostname` needs to be set as null-pointer after free in `cifs_put_tcp_session` function, or when `cifsd` thread attempts to resolve hostname and reconnect the host, the thread would deref the invalid pointer. Here is one of practical backtrace examples as reference: Task 477 --------------------------- do_mount path_mount do_new_mount vfs_get_tree smb3_get_tree smb3_get_tree_common cifs_smb3_do_mount cifs_mount mount_put_conns cifs_put_tcp_session --> kfree(server->hostname) cifsd --------------------------- kthread cifs_demultiplex_thread cifs_reconnect reconn_set_ipaddr_from_hostname --> if (!server->hostname) --> if (server->hostname[0] == '\0') // !! UAF fault here CIFS: VFS: cifs_mount failed w/return code = -112 mount error(112): Host is down BUG: KASAN: use-after-free in reconn_set_ipaddr_from_hostname+0x2ba/0x310 Read of size 1 at addr ffff888108f35380 by task cifsd/480 CPU: 2 PID: 480 Comm: cifsd Not tainted 6.1.0-rc2-00106-gf705792f89dd-dirty #25 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x68/0x85 print_report+0x16c/0x4a3 kasan_report+0x95/0x190 reconn_set_ipaddr_from_hostname+0x2ba/0x310 __cifs_reconnect.part.0+0x241/0x800 cifs_reconnect+0x65f/0xb60 cifs_demultiplex_thread+0x1570/0x2570 kthread+0x2c5/0x380 ret_from_fork+0x22/0x30 </TASK> Allocated by task 477: kasan_save_stack+0x1e/0x40 kasan_set_track+0x21/0x30 __kasan_kmalloc+0x7e/0x90 __kmalloc_node_track_caller+0x52/0x1b0 kstrdup+0x3b/0x70 cifs_get_tcp_session+0xbc/0x19b0 mount_get_conns+0xa9/0x10c0 cifs_mount+0xdf/0x1970 cifs_smb3_do_mount+0x295/0x1660 smb3_get_tree+0x352/0x5e0 vfs_get_tree+0x8e/0x2e0 path_mount+0xf8c/0x1990 do_mount+0xee/0x110 __x64_sys_mount+0x14b/0x1f0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd Freed by task 477: kasan_save_stack+0x1e/0x40 kasan_set_track+0x21/0x30 kasan_save_free_info+0x2a/0x50 __kasan_slab_free+0x10a/0x190 __kmem_cache_free+0xca/0x3f0 cifs_put_tcp_session+0x30c/0x450 cifs_mount+0xf95/0x1970 cifs_smb3_do_mount+0x295/0x1660 smb3_get_tree+0x352/0x5e0 vfs_get_tree+0x8e/0x2e0 path_mount+0xf8c/0x1990 do_mount+0xee/0x110 __x64_sys_mount+0x14b/0x1f0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd The buggy address belongs to the object at ffff888108f35380 which belongs to the cache kmalloc-16 of size 16 The buggy address is located 0 bytes inside of 16-byte region [ffff888108f35380, ffff888108f35390) The buggy address belongs to the physical page: page:00000000333f8e58 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888108f350e0 pfn:0x108f35 flags: 0x200000000000200(slab|node=0|zone=2) raw: 0200000000000200 0000000000000000 dead000000000122 ffff8881000423c0 raw: ffff888108f350e0 000000008080007a 00000001ffffffff 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff888108f35280: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc ffff888108f35300: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc >ffff888108f35380: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc ^ ffff888108f35400: fa fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff888108f35480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc Fixes: 7be3248f3139 ("cifs: To match file servers, make sure the server hostname matches") Signed-off-by: Zeng Heng <zengheng4@huawei.com> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- changes in v4: - correct fix tag - add reviewed-by --- fs/cifs/connect.c | 1 + 1 file changed, 1 insertion(+)