Skip to content

Commit 24a9799

Browse files
pcacjrSteve French
authored and
Steve French
committed
smb: client: fix UAF in smb2_reconnect_server()
The UAF bug is due to smb2_reconnect_server() accessing a session that is already being teared down by another thread that is executing __cifs_put_smb_ses(). This can happen when (a) the client has connection to the server but no session or (b) another thread ends up setting @ses->ses_status again to something different than SES_EXITING. To fix this, we need to make sure to unconditionally set @ses->ses_status to SES_EXITING and prevent any other threads from setting a new status while we're still tearing it down. The following can be reproduced by adding some delay to right after the ipc is freed in __cifs_put_smb_ses() - which will give smb2_reconnect_server() worker a chance to run and then accessing @ses->ipc: kinit ... mount.cifs //srv/share /mnt/1 -o sec=krb5,nohandlecache,echo_interval=10 [disconnect srv] ls /mnt/1 &>/dev/null sleep 30 kdestroy [reconnect srv] sleep 10 umount /mnt/1 ... CIFS: VFS: Verify user has a krb5 ticket and keyutils is installed CIFS: VFS: \\srv Send error in SessSetup = -126 CIFS: VFS: Verify user has a krb5 ticket and keyutils is installed CIFS: VFS: \\srv Send error in SessSetup = -126 general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI CPU: 3 PID: 50 Comm: kworker/3:1 Not tainted 6.9.0-rc2 #1 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-1.fc39 04/01/2014 Workqueue: cifsiod smb2_reconnect_server [cifs] RIP: 0010:__list_del_entry_valid_or_report+0x33/0xf0 Code: 4f 08 48 85 d2 74 42 48 85 c9 74 59 48 b8 00 01 00 00 00 00 ad de 48 39 c2 74 61 48 b8 22 01 00 00 00 00 74 69 <48> 8b 01 48 39 f8 75 7b 48 8b 72 08 48 39 c6 0f 85 88 00 00 00 b8 RSP: 0018:ffffc900001bfd70 EFLAGS: 00010a83 RAX: dead000000000122 RBX: ffff88810da53838 RCX: 6b6b6b6b6b6b6b6b RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc02f6878 RDI: ffff88810da53800 RBP: ffff88810da53800 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000001 R12: ffff88810c064000 R13: 0000000000000001 R14: ffff88810c064000 R15: ffff8881039cc000 FS: 0000000000000000(0000) GS:ffff888157c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fe3728b1000 CR3: 000000010caa4000 CR4: 0000000000750ef0 PKRU: 55555554 Call Trace: <TASK> ? die_addr+0x36/0x90 ? exc_general_protection+0x1c1/0x3f0 ? asm_exc_general_protection+0x26/0x30 ? __list_del_entry_valid_or_report+0x33/0xf0 __cifs_put_smb_ses+0x1ae/0x500 [cifs] smb2_reconnect_server+0x4ed/0x710 [cifs] process_one_work+0x205/0x6b0 worker_thread+0x191/0x360 ? __pfx_worker_thread+0x10/0x10 kthread+0xe2/0x110 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x34/0x50 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> Cc: stable@vger.kernel.org Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent ebd9779 commit 24a9799

File tree

1 file changed

+34
-49
lines changed

1 file changed

+34
-49
lines changed

fs/smb/client/connect.c

Lines changed: 34 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
232232

233233
spin_lock(&cifs_tcp_ses_lock);
234234
list_for_each_entry_safe(ses, nses, &pserver->smb_ses_list, smb_ses_list) {
235-
/* check if iface is still active */
235+
spin_lock(&ses->ses_lock);
236+
if (ses->ses_status == SES_EXITING) {
237+
spin_unlock(&ses->ses_lock);
238+
continue;
239+
}
240+
spin_unlock(&ses->ses_lock);
241+
236242
spin_lock(&ses->chan_lock);
237243
if (cifs_ses_get_chan_index(ses, server) ==
238244
CIFS_INVAL_CHAN_INDEX) {
@@ -1963,31 +1969,6 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb3_fs_context *ctx)
19631969
return rc;
19641970
}
19651971

1966-
/**
1967-
* cifs_free_ipc - helper to release the session IPC tcon
1968-
* @ses: smb session to unmount the IPC from
1969-
*
1970-
* Needs to be called everytime a session is destroyed.
1971-
*
1972-
* On session close, the IPC is closed and the server must release all tcons of the session.
1973-
* No need to send a tree disconnect here.
1974-
*
1975-
* Besides, it will make the server to not close durable and resilient files on session close, as
1976-
* specified in MS-SMB2 3.3.5.6 Receiving an SMB2 LOGOFF Request.
1977-
*/
1978-
static int
1979-
cifs_free_ipc(struct cifs_ses *ses)
1980-
{
1981-
struct cifs_tcon *tcon = ses->tcon_ipc;
1982-
1983-
if (tcon == NULL)
1984-
return 0;
1985-
1986-
tconInfoFree(tcon);
1987-
ses->tcon_ipc = NULL;
1988-
return 0;
1989-
}
1990-
19911972
static struct cifs_ses *
19921973
cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
19931974
{
@@ -2019,48 +2000,52 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
20192000
void __cifs_put_smb_ses(struct cifs_ses *ses)
20202001
{
20212002
struct TCP_Server_Info *server = ses->server;
2003+
struct cifs_tcon *tcon;
20222004
unsigned int xid;
20232005
size_t i;
2006+
bool do_logoff;
20242007
int rc;
20252008

2009+
spin_lock(&cifs_tcp_ses_lock);
20262010
spin_lock(&ses->ses_lock);
2027-
if (ses->ses_status == SES_EXITING) {
2011+
cifs_dbg(FYI, "%s: id=0x%llx ses_count=%d ses_status=%u ipc=%s\n",
2012+
__func__, ses->Suid, ses->ses_count, ses->ses_status,
2013+
ses->tcon_ipc ? ses->tcon_ipc->tree_name : "none");
2014+
if (ses->ses_status == SES_EXITING || --ses->ses_count > 0) {
20282015
spin_unlock(&ses->ses_lock);
2016+
spin_unlock(&cifs_tcp_ses_lock);
20292017
return;
20302018
}
2031-
spin_unlock(&ses->ses_lock);
2019+
/* ses_count can never go negative */
2020+
WARN_ON(ses->ses_count < 0);
20322021

2033-
cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count);
2034-
cifs_dbg(FYI,
2035-
"%s: ses ipc: %s\n", __func__, ses->tcon_ipc ? ses->tcon_ipc->tree_name : "NONE");
2022+
spin_lock(&ses->chan_lock);
2023+
cifs_chan_clear_need_reconnect(ses, server);
2024+
spin_unlock(&ses->chan_lock);
20362025

2037-
spin_lock(&cifs_tcp_ses_lock);
2038-
if (--ses->ses_count > 0) {
2039-
spin_unlock(&cifs_tcp_ses_lock);
2040-
return;
2041-
}
2042-
spin_lock(&ses->ses_lock);
2043-
if (ses->ses_status == SES_GOOD)
2044-
ses->ses_status = SES_EXITING;
2026+
do_logoff = ses->ses_status == SES_GOOD && server->ops->logoff;
2027+
ses->ses_status = SES_EXITING;
2028+
tcon = ses->tcon_ipc;
2029+
ses->tcon_ipc = NULL;
20452030
spin_unlock(&ses->ses_lock);
20462031
spin_unlock(&cifs_tcp_ses_lock);
20472032

2048-
/* ses_count can never go negative */
2049-
WARN_ON(ses->ses_count < 0);
2050-
2051-
spin_lock(&ses->ses_lock);
2052-
if (ses->ses_status == SES_EXITING && server->ops->logoff) {
2053-
spin_unlock(&ses->ses_lock);
2054-
cifs_free_ipc(ses);
2033+
/*
2034+
* On session close, the IPC is closed and the server must release all
2035+
* tcons of the session. No need to send a tree disconnect here.
2036+
*
2037+
* Besides, it will make the server to not close durable and resilient
2038+
* files on session close, as specified in MS-SMB2 3.3.5.6 Receiving an
2039+
* SMB2 LOGOFF Request.
2040+
*/
2041+
tconInfoFree(tcon);
2042+
if (do_logoff) {
20552043
xid = get_xid();
20562044
rc = server->ops->logoff(xid, ses);
20572045
if (rc)
20582046
cifs_server_dbg(VFS, "%s: Session Logoff failure rc=%d\n",
20592047
__func__, rc);
20602048
_free_xid(xid);
2061-
} else {
2062-
spin_unlock(&ses->ses_lock);
2063-
cifs_free_ipc(ses);
20642049
}
20652050

20662051
spin_lock(&cifs_tcp_ses_lock);

0 commit comments

Comments
 (0)