| 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
| ||
|
Message-ID: <46fbef33-13a3-3909-0345-628e98a5b460@huawei.com>
Date: Sat, 29 Oct 2022 10:54:52 +0800
From: Zeng Heng <zengheng4@...wei.com>
To: Shyam Prasad N <nspmangalore@...il.com>
CC: <sfrench@...ba.org>, <tom@...pey.com>, <sprasad@...rosoft.com>,
<pc@....nz>, <lsahlber@...hat.com>, <linux-cifs@...r.kernel.org>,
<samba-technical@...ts.samba.org>, <linux-kernel@...r.kernel.org>,
<liwei391@...wei.com>
Subject: Re: [PATCH v4] cifs: fix use-after-free caused by invalid pointer
`hostname`
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@...wei.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@...wei.com>
>> Reviewed-by: Paulo Alcantara (SUSE) <pc@....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.
>
Powered by blists - more mailing lists