FAILED: patch "[PATCH] smb: client: fix UAF in smb2_reconnect_server()" failed to apply to 6.1-stable tree
The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x 24a9799aa8efecd0eb55a75e35f9d8e6400063aa # <resolve conflicts, build, test, etc.> git commit -s git send-email --to 'stable@vger.kernel.org' --in-reply-to '2024040834-magazine-audience-8aa4@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
Possible dependencies:
24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") 7257bcf3bdc7 ("cifs: cifs_chan_is_iface_active should be called with chan_lock held") 27e1fd343f80 ("cifs: after disabling multichannel, mark tcon for reconnect") fa1d0508bdd4 ("cifs: account for primary channel in the interface list") a6d8fb54a515 ("cifs: distribute channels across interfaces based on speed") c37ed2d7d098 ("smb: client: remove extra @chan_count check in __cifs_put_smb_ses()") ff7d80a9f271 ("cifs: fix session state transition to avoid use-after-free issue") 38c8a9a52082 ("smb: move client and server files to common directory fs/smb") 943fb67b0902 ("cifs: missing lock when updating session status") bc962159e8e3 ("cifs: avoid race conditions with parallel reconnects") 1bcd548d935a ("cifs: prevent data race in cifs_reconnect_tcon()") e77978de4765 ("cifs: update ip_addr for ses only for primary chan setup") 3c0070f54b31 ("cifs: prevent data race in smb2_reconnect()") 05844bd661d9 ("cifs: print last update time for interface list") 25cf01b7c920 ("cifs: set correct status of tcon ipc when reconnecting") abdb1742a312 ("cifs: get rid of mount options string parsing") 9fd29a5bae6e ("cifs: use fs_context for automounts")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 24a9799aa8efecd0eb55a75e35f9d8e6400063aa Mon Sep 17 00:00:00 2001 From: Paulo Alcantara pc@manguebit.com Date: Mon, 1 Apr 2024 14:13:10 -0300 Subject: [PATCH] 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
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 9b85b5341822..ee29bc57300c 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -232,7 +232,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
spin_lock(&cifs_tcp_ses_lock); list_for_each_entry_safe(ses, nses, &pserver->smb_ses_list, smb_ses_list) { - /* check if iface is still active */ + spin_lock(&ses->ses_lock); + if (ses->ses_status == SES_EXITING) { + spin_unlock(&ses->ses_lock); + continue; + } + spin_unlock(&ses->ses_lock); + spin_lock(&ses->chan_lock); if (cifs_ses_get_chan_index(ses, server) == CIFS_INVAL_CHAN_INDEX) { @@ -1963,31 +1969,6 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb3_fs_context *ctx) return rc; }
-/** - * cifs_free_ipc - helper to release the session IPC tcon - * @ses: smb session to unmount the IPC from - * - * Needs to be called everytime a session is destroyed. - * - * On session close, the IPC is closed and the server must release all tcons of the session. - * No need to send a tree disconnect here. - * - * Besides, it will make the server to not close durable and resilient files on session close, as - * specified in MS-SMB2 3.3.5.6 Receiving an SMB2 LOGOFF Request. - */ -static int -cifs_free_ipc(struct cifs_ses *ses) -{ - struct cifs_tcon *tcon = ses->tcon_ipc; - - if (tcon == NULL) - return 0; - - tconInfoFree(tcon); - ses->tcon_ipc = NULL; - return 0; -} - static struct cifs_ses * cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx) { @@ -2019,48 +2000,52 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx) void __cifs_put_smb_ses(struct cifs_ses *ses) { struct TCP_Server_Info *server = ses->server; + struct cifs_tcon *tcon; unsigned int xid; size_t i; + bool do_logoff; int rc;
- spin_lock(&ses->ses_lock); - if (ses->ses_status == SES_EXITING) { - spin_unlock(&ses->ses_lock); - return; - } - spin_unlock(&ses->ses_lock); - - cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count); - cifs_dbg(FYI, - "%s: ses ipc: %s\n", __func__, ses->tcon_ipc ? ses->tcon_ipc->tree_name : "NONE"); - spin_lock(&cifs_tcp_ses_lock); - if (--ses->ses_count > 0) { + spin_lock(&ses->ses_lock); + cifs_dbg(FYI, "%s: id=0x%llx ses_count=%d ses_status=%u ipc=%s\n", + __func__, ses->Suid, ses->ses_count, ses->ses_status, + ses->tcon_ipc ? ses->tcon_ipc->tree_name : "none"); + if (ses->ses_status == SES_EXITING || --ses->ses_count > 0) { + spin_unlock(&ses->ses_lock); spin_unlock(&cifs_tcp_ses_lock); return; } - spin_lock(&ses->ses_lock); - if (ses->ses_status == SES_GOOD) - ses->ses_status = SES_EXITING; - spin_unlock(&ses->ses_lock); - spin_unlock(&cifs_tcp_ses_lock); - /* ses_count can never go negative */ WARN_ON(ses->ses_count < 0);
- spin_lock(&ses->ses_lock); - if (ses->ses_status == SES_EXITING && server->ops->logoff) { - spin_unlock(&ses->ses_lock); - cifs_free_ipc(ses); + spin_lock(&ses->chan_lock); + cifs_chan_clear_need_reconnect(ses, server); + spin_unlock(&ses->chan_lock); + + do_logoff = ses->ses_status == SES_GOOD && server->ops->logoff; + ses->ses_status = SES_EXITING; + tcon = ses->tcon_ipc; + ses->tcon_ipc = NULL; + spin_unlock(&ses->ses_lock); + spin_unlock(&cifs_tcp_ses_lock); + + /* + * On session close, the IPC is closed and the server must release all + * tcons of the session. No need to send a tree disconnect here. + * + * Besides, it will make the server to not close durable and resilient + * files on session close, as specified in MS-SMB2 3.3.5.6 Receiving an + * SMB2 LOGOFF Request. + */ + tconInfoFree(tcon); + if (do_logoff) { xid = get_xid(); rc = server->ops->logoff(xid, ses); if (rc) cifs_server_dbg(VFS, "%s: Session Logoff failure rc=%d\n", __func__, rc); _free_xid(xid); - } else { - spin_unlock(&ses->ses_lock); - cifs_free_ipc(ses); }
spin_lock(&cifs_tcp_ses_lock);
Hi Paulo, hi Steve,
On Mon, Apr 08, 2024 at 12:19:35PM +0200, gregkh@linuxfoundation.org wrote:
The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x 24a9799aa8efecd0eb55a75e35f9d8e6400063aa # <resolve conflicts, build, test, etc.> git commit -s git send-email --to 'stable@vger.kernel.org' --in-reply-to '2024040834-magazine-audience-8aa4@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
Possible dependencies:
24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") 7257bcf3bdc7 ("cifs: cifs_chan_is_iface_active should be called with chan_lock held") 27e1fd343f80 ("cifs: after disabling multichannel, mark tcon for reconnect") fa1d0508bdd4 ("cifs: account for primary channel in the interface list") a6d8fb54a515 ("cifs: distribute channels across interfaces based on speed") c37ed2d7d098 ("smb: client: remove extra @chan_count check in __cifs_put_smb_ses()") ff7d80a9f271 ("cifs: fix session state transition to avoid use-after-free issue") 38c8a9a52082 ("smb: move client and server files to common directory fs/smb") 943fb67b0902 ("cifs: missing lock when updating session status") bc962159e8e3 ("cifs: avoid race conditions with parallel reconnects") 1bcd548d935a ("cifs: prevent data race in cifs_reconnect_tcon()") e77978de4765 ("cifs: update ip_addr for ses only for primary chan setup") 3c0070f54b31 ("cifs: prevent data race in smb2_reconnect()") 05844bd661d9 ("cifs: print last update time for interface list") 25cf01b7c920 ("cifs: set correct status of tcon ipc when reconnecting") abdb1742a312 ("cifs: get rid of mount options string parsing") 9fd29a5bae6e ("cifs: use fs_context for automounts")
In Debian we got a report yhsy in s CIFS (DFS) infrastructure and after mounting at some point later but reproducible they are able to trigger within few minutes a system hang with a trace:
CIFS: VFS: \SOME.SERVER.FQDN cifs_put_smb_ses: Session Logoff failure rc=-11 CIFS: VFS: \(null) cifs_put_smb_ses: Session Logoff failure rc=-11 list_del corruption, ffff966536fe7800->next is NULL ------------[ cut here ]------------ kernel BUG at lib/list_debug.c:49! invalid opcode: 0000 [#1] PREEMPT SMP PTI CPU: 6 PID: 2498151 Comm: kworker/6:9 Tainted: G OE 6.1.0-23-amd64 #1 Debian 6.1.99-1 Hardware name: Dell Inc. PowerEdge R620/0KCKR5, BIOS 2.9.0 12/06/2019 Workqueue: events delayed_mntput RIP: 0010:__list_del_entry_valid.cold+0xf/0x6f Code: c7 c7 88 3c fa a0 e8 90 a0 fe ff 0f 0b 48 c7 c7 60 3c fa a0 e8 82 a0 fe ff 0f 0b 48 89 fe 48 c7 c7 70 3d fa a0 e8 71 a0 fe ff <0f> 0b 48 89 d1 48 c7 c7 90 3e fa a0 48 89 c2 e8 5d a0 fe ff 0f 0b RSP: 0018:ffffad83a63f7dd0 EFLAGS: 00010246 RAX: 0000000000000033 RBX: ffff966536fe7800 RCX: 0000000000000027 RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff965e7f8e03a0 RBP: 00000000142d66a6 R08: 0000000000000000 R09: ffffad83a63f7c68 R10: 0000000000000003 R11: ffff966ebff11be0 R12: 00000000fffffff5 R13: ffff966536fe7000 R14: ffff966536fe7020 R15: ffffffffa1770b88 FS: 0000000000000000(0000) GS:ffff965e7f8c0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fe35dbcb7b0 CR3: 0000000f36c10001 CR4: 00000000000606e0 Call Trace: <TASK> ? __die_body.cold+0x1a/0x1f ? die+0x2a/0x50 ? do_trap+0xc5/0x110 ? __list_del_entry_valid.cold+0xf/0x6f ? do_error_trap+0x6a/0x90 ? __list_del_entry_valid.cold+0xf/0x6f ? exc_invalid_op+0x4c/0x60 ? __list_del_entry_valid.cold+0xf/0x6f ? asm_exc_invalid_op+0x16/0x20 ? __list_del_entry_valid.cold+0xf/0x6f cifs_put_smb_ses+0xbb/0x3e0 [cifs] mount_group_release+0x82/0xa0 [cifs] cifs_umount+0x88/0xa0 [cifs] deactivate_locked_super+0x2f/0xa0 cleanup_mnt+0xbd/0x150 delayed_mntput+0x28/0x40 process_one_work+0x1c7/0x380 worker_thread+0x4d/0x380 ? rescuer_thread+0x3a0/0x3a0 kthread+0xda/0x100 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork+0x22/0x30 </TASK> Modules linked in: bluetooth jitterentropy_rng drbg ansi_cprng ecdh_generic rfkill ecc overlay isofs cmac nls_utf8 cifs cifs_arc4 cifs_md4 rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache netfs tls beegfs(OE) rpcrdma rdma_ucm ib_iser rdma_cm iw_cm ib_cm libiscsi scsi_transport_iscsi rdma_rxe ib_uverbs ip6_udp_tunnel udp_tunnel ib_core nft_chain_nat xt_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nft_compat nf_tables nfnetlink intel_rapl_msr intel_rapl_common sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel ipmi_ssif binfmt_misc kvm irqbypass ghash_clmulni_intel sha512_ssse3 sha512_generic sha256_ssse3 sha1_ssse3 aesni_intel crypto_simd cryptd rapl dcdbas mgag200 intel_cstate joydev evdev drm_shmem_helper intel_uncore iTCO_wdt ipmi_si drm_kms_helper mei_me intel_pmc_bxt ipmi_devintf iTCO_vendor_support pcspkr i2c_algo_bit mei ipmi_msghandler watchdog sg acpi_power_meter button nfsd auth_rpcgss nfs_acl lockd grace sunrpc drm fuse loop efi_pstore configfs ip_tables x_tables autofs4 ext4 crc16 mbcache jbd2 dm_mod hid_generic usbhid hid sd_mod t10_pi sr_mod cdrom crc64_rocksoft crc64 crc_t10dif crct10dif_generic ahci libahci crct10dif_pclmul crct10dif_common crc32_pclmul libata ehci_pci bnx2x ehci_hcd megaraid_sas usbcore scsi_mod lpc_ich usb_common mdio libcrc32c crc32c_generic scsi_common crc32c_intel wmi ---[ end trace 0000000000000000 ]--- RIP: 0010:__list_del_entry_valid.cold+0xf/0x6f Code: c7 c7 88 3c fa a0 e8 90 a0 fe ff 0f 0b 48 c7 c7 60 3c fa a0 e8 82 a0 fe ff 0f 0b 48 89 fe 48 c7 c7 70 3d fa a0 e8 71 a0 fe ff <0f> 0b 48 89 d1 48 c7 c7 90 3e fa a0 48 89 c2 e8 5d a0 fe ff 0f 0b RSP: 0018:ffffad83a63f7dd0 EFLAGS: 00010246 RAX: 0000000000000033 RBX: ffff966536fe7800 RCX: 0000000000000027 RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff965e7f8e03a0 RBP: 00000000142d66a6 R08: 0000000000000000 R09: ffffad83a63f7c68 R10: 0000000000000003 R11: ffff966ebff11be0 R12: 00000000fffffff5 R13: ffff966536fe7000 R14: ffff966536fe7020 R15: ffffffffa1770b88 FS: 0000000000000000(0000) GS:ffff965e7f8c0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fe35dbcb7b0 CR3: 0000000f36c10001 CR4: 00000000000606e0 note: kworker/6:9[2498151] exited with preempt_count 1
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Regards, Salvatore
Hi *,
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
cheers Michael
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Regards, Salvatore
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
From 411fb6398fe3c3c08a000d717bff189f08d2041c Mon Sep 17 00:00:00 2001 From: Paulo Alcantara pc@manguebit.com Date: Mon, 1 Apr 2024 14:13:10 -0300 Subject: [PATCH] smb: client: fix UAF in smb2_reconnect_server()
commit 24a9799aa8efecd0eb55a75e35f9d8e6400063aa upstream.
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 [Michael Krause: Naive, manual merge because the 3rd hunk would not apply] Signed-off-by: Michael Krause mk-debian@galax.is --- fs/smb/client/connect.c | 80 ++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 45 deletions(-)
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 87ce71b39b77..20c50736456a 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -259,7 +259,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
spin_lock(&cifs_tcp_ses_lock); list_for_each_entry_safe(ses, nses, &pserver->smb_ses_list, smb_ses_list) { - /* check if iface is still active */ + spin_lock(&ses->ses_lock); + if (ses->ses_status == SES_EXITING) { + spin_unlock(&ses->ses_lock); + continue; + } + spin_unlock(&ses->ses_lock); + spin_lock(&ses->chan_lock); if (!cifs_chan_is_iface_active(ses, server)) { spin_unlock(&ses->chan_lock); @@ -1977,31 +1983,6 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb3_fs_context *ctx) return rc; }
-/** - * cifs_free_ipc - helper to release the session IPC tcon - * @ses: smb session to unmount the IPC from - * - * Needs to be called everytime a session is destroyed. - * - * On session close, the IPC is closed and the server must release all tcons of the session. - * No need to send a tree disconnect here. - * - * Besides, it will make the server to not close durable and resilient files on session close, as - * specified in MS-SMB2 3.3.5.6 Receiving an SMB2 LOGOFF Request. - */ -static int -cifs_free_ipc(struct cifs_ses *ses) -{ - struct cifs_tcon *tcon = ses->tcon_ipc; - - if (tcon == NULL) - return 0; - - tconInfoFree(tcon); - ses->tcon_ipc = NULL; - return 0; -} - static struct cifs_ses * cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx) { @@ -2035,35 +2016,44 @@ void cifs_put_smb_ses(struct cifs_ses *ses) { unsigned int rc, xid; unsigned int chan_count; + bool do_logoff; + struct cifs_tcon *tcon; struct TCP_Server_Info *server = ses->server;
+ spin_lock(&cifs_tcp_ses_lock); spin_lock(&ses->ses_lock); - if (ses->ses_status == SES_EXITING) { cifs_dbg(FYI, "%s: id=0x%llx ses_count=%d ses_status=%u ipc=%s\n", + __func__, ses->Suid, ses->ses_count, ses->ses_status, + ses->tcon_ipc ? ses->tcon_ipc->tree_name : "none"); + if (ses->ses_status == SES_EXITING || --ses->ses_count > 0) { spin_unlock(&ses->ses_lock); + spin_unlock(&cifs_tcp_ses_lock); return; } - spin_unlock(&ses->ses_lock); + /* ses_count can never go negative */ + WARN_ON(ses->ses_count < 0);
- cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count); - cifs_dbg(FYI, - "%s: ses ipc: %s\n", __func__, ses->tcon_ipc ? ses->tcon_ipc->tree_name : "NONE"); + spin_lock(&ses->chan_lock); + cifs_chan_clear_need_reconnect(ses, server); + spin_unlock(&ses->chan_lock);
- spin_lock(&cifs_tcp_ses_lock); - if (--ses->ses_count > 0) { - spin_unlock(&cifs_tcp_ses_lock); - return; - } + do_logoff = ses->ses_status == SES_GOOD && server->ops->logoff; + ses->ses_status = SES_EXITING; + tcon = ses->tcon_ipc; + ses->tcon_ipc = NULL; + spin_unlock(&ses->ses_lock); spin_unlock(&cifs_tcp_ses_lock);
- /* ses_count can never go negative */ - WARN_ON(ses->ses_count < 0); - - if (ses->ses_status == SES_GOOD) - ses->ses_status = SES_EXITING; - - cifs_free_ipc(ses); - - if (ses->ses_status == SES_EXITING && server->ops->logoff) { + /* + * On session close, the IPC is closed and the server must release all + * tcons of the session. No need to send a tree disconnect here. + * + * Besides, it will make the server to not close durable and resilient + * files on session close, as specified in MS-SMB2 3.3.5.6 Receiving an + * SMB2 LOGOFF Request. + */ + tconInfoFree(tcon); + if (do_logoff) { xid = get_xid(); rc = server->ops->logoff(xid, ses); if (rc)
On Tue, Dec 10, 2024 at 12:05:00AM +0100, Michael Krause wrote:
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
From 411fb6398fe3c3c08a000d717bff189f08d2041c Mon Sep 17 00:00:00 2001 From: Paulo Alcantara pc@manguebit.com Date: Mon, 1 Apr 2024 14:13:10 -0300 Subject: [PATCH] smb: client: fix UAF in smb2_reconnect_server()
commit 24a9799aa8efecd0eb55a75e35f9d8e6400063aa upstream.
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 [Michael Krause: Naive, manual merge because the 3rd hunk would not apply] Signed-off-by: Michael Krause mk-debian@galax.is
fs/smb/client/connect.c | 80 ++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 45 deletions(-)
What kernel(s) is this commit supposed to be for?
thanks,
greg k-h
Hi Greg,
On Tue, Dec 10, 2024 at 09:51:58AM +0100, Greg KH wrote:
On Tue, Dec 10, 2024 at 12:05:00AM +0100, Michael Krause wrote:
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
From 411fb6398fe3c3c08a000d717bff189f08d2041c Mon Sep 17 00:00:00 2001 From: Paulo Alcantara pc@manguebit.com Date: Mon, 1 Apr 2024 14:13:10 -0300 Subject: [PATCH] smb: client: fix UAF in smb2_reconnect_server()
commit 24a9799aa8efecd0eb55a75e35f9d8e6400063aa upstream.
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 [Michael Krause: Naive, manual merge because the 3rd hunk would not apply] Signed-off-by: Michael Krause mk-debian@galax.is
fs/smb/client/connect.c | 80 ++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 45 deletions(-)
What kernel(s) is this commit supposed to be for?
6.1.y (at least, possibly older, but the reporter did test on 6.1.y for Debian, context from https://bugs.debian.org/1088733). Upper stable series have already the commit in 6.6.29, 6.8.5.
Regards, Salvatore
On Tue, Dec 10, 2024 at 12:05:00AM +0100, Michael Krause wrote:
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
It's corrupted somehow:
patching file fs/smb/client/connect.c patch: **** malformed patch at line 202: if (rc)
Can you resend it or attach it?
thanks,
greg k-h
On 12/12/24 1:26 PM, Greg KH wrote:
On Tue, Dec 10, 2024 at 12:05:00AM +0100, Michael Krause wrote:
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote:
Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()") which seems in fact to solve the issue.
Michael, can you please post your backport here for review from Paulo and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
It's corrupted somehow:
patching file fs/smb/client/connect.c patch: **** malformed patch at line 202: if (rc)
Can you resend it or attach it?
thanks,
greg k-h
Ugh, how embarrassing. I'm sorry, I "fixed" some minor whitespace issue directly in the patch and apparently did something wrong.
I redid the white space fix before diffing again and attach and inline the new version. The chunks are a bit alternated to the earlier version now unfortunately. This one applies..
From 411fb6398fe3c3c08a000d717bff189f08d2041c Mon Sep 17 00:00:00 2001 From: Paulo Alcantara pc@manguebit.com Date: Mon, 1 Apr 2024 14:13:10 -0300 Subject: [PATCH] smb: client: fix UAF in smb2_reconnect_server()
commit 24a9799aa8efecd0eb55a75e35f9d8e6400063aa upstream.
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 [Michael Krause: Naive, manual merge because the 3rd hunk would not apply.] Signed-off-by: Michael Krause mk-debian@galax.is --- fs/smb/client/connect.c | 80 ++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 45 deletions(-)
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 87ce71b39b77..20c50736456a 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -259,7 +259,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(st
spin_lock(&cifs_tcp_ses_lock); list_for_each_entry_safe(ses, nses, &pserver->smb_ses_list, smb_ses_list) { - /* check if iface is still active */ + spin_lock(&ses->ses_lock); + if (ses->ses_status == SES_EXITING) { + spin_unlock(&ses->ses_lock); + continue; + } + spin_unlock(&ses->ses_lock); + spin_lock(&ses->chan_lock); if (!cifs_chan_is_iface_active(ses, server)) { spin_unlock(&ses->chan_lock); @@ -1977,31 +1983,6 @@ out: return rc; }
-/** - * cifs_free_ipc - helper to release the session IPC tcon - * @ses: smb session to unmount the IPC from - * - * Needs to be called everytime a session is destroyed. - * - * On session close, the IPC is closed and the server must release all tcons of the session. - * No need to send a tree disconnect here. - * - * Besides, it will make the server to not close durable and resilient files on session close, as - * specified in MS-SMB2 3.3.5.6 Receiving an SMB2 LOGOFF Request. - */ -static int -cifs_free_ipc(struct cifs_ses *ses) -{ - struct cifs_tcon *tcon = ses->tcon_ipc; - - if (tcon == NULL) - return 0; - - tconInfoFree(tcon); - ses->tcon_ipc = NULL; - return 0; -} - static struct cifs_ses * cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx) { @@ -2035,35 +2016,44 @@ void cifs_put_smb_ses(struct cifs_ses *s { unsigned int rc, xid; unsigned int chan_count; + bool do_logoff; + struct cifs_tcon *tcon; struct TCP_Server_Info *server = ses->server;
+ spin_lock(&cifs_tcp_ses_lock); spin_lock(&ses->ses_lock); - if (ses->ses_status == SES_EXITING) { + cifs_dbg(FYI, "%s: id=0x%llx ses_count=%d ses_status=%u ipc=%s\n", + __func__, ses->Suid, ses->ses_count, ses->ses_status, + ses->tcon_ipc ? ses->tcon_ipc->tree_name : "none"); + if (ses->ses_status == SES_EXITING || --ses->ses_count > 0) { spin_unlock(&ses->ses_lock); - return; - } - spin_unlock(&ses->ses_lock); - - cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count); - cifs_dbg(FYI, - "%s: ses ipc: %s\n", __func__, ses->tcon_ipc ? ses->tcon_ipc->tree_name : "NONE"); - - spin_lock(&cifs_tcp_ses_lock); - if (--ses->ses_count > 0) { spin_unlock(&cifs_tcp_ses_lock); return; } - spin_unlock(&cifs_tcp_ses_lock); - /* ses_count can never go negative */ WARN_ON(ses->ses_count < 0);
- if (ses->ses_status == SES_GOOD) - ses->ses_status = SES_EXITING; - - cifs_free_ipc(ses); + spin_lock(&ses->chan_lock); + cifs_chan_clear_need_reconnect(ses, server); + spin_unlock(&ses->chan_lock); + + do_logoff = ses->ses_status == SES_GOOD && server->ops->logoff; + ses->ses_status = SES_EXITING; + tcon = ses->tcon_ipc; + ses->tcon_ipc = NULL; + spin_unlock(&ses->ses_lock); + spin_unlock(&cifs_tcp_ses_lock);
- if (ses->ses_status == SES_EXITING && server->ops->logoff) { + /* + * On session close, the IPC is closed and the server must release all + * tcons of the session. No need to send a tree disconnect here. + * + * Besides, it will make the server to not close durable and resilient + * files on session close, as specified in MS-SMB2 3.3.5.6 Receiving an + * SMB2 LOGOFF Request. + */ + tconInfoFree(tcon); + if (do_logoff) { xid = get_xid(); rc = server->ops->logoff(xid, ses); if (rc)
On Thu, Dec 12, 2024 at 10:48:55PM +0100, Michael Krause wrote:
On 12/12/24 1:26 PM, Greg KH wrote:
On Tue, Dec 10, 2024 at 12:05:00AM +0100, Michael Krause wrote:
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote: > Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF > in smb2_reconnect_server()") which seems in fact to solve the issue. > > Michael, can you please post your backport here for review from Paulo > and Steve?
Of course, attached.
Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
It's corrupted somehow:
patching file fs/smb/client/connect.c patch: **** malformed patch at line 202: if (rc)
Can you resend it or attach it?
thanks,
greg k-h
Ugh, how embarrassing. I'm sorry, I "fixed" some minor whitespace issue directly in the patch and apparently did something wrong.
I redid the white space fix before diffing again and attach and inline the new version. The chunks are a bit alternated to the earlier version now unfortunately. This one applies..
Doesn't apply for me:
checking file fs/smb/client/connect.c Hunk #1 FAILED at 259. Hunk #2 FAILED at 1977. Hunk #3 FAILED at 2035. 3 out of 3 hunks FAILED checking file fs/smb/client/connect.c
Any ideas?
thanks,
greg k-h
Hi Greg,
On Fri, Dec 13, 2024 at 03:33:31PM +0100, Greg KH wrote:
On Thu, Dec 12, 2024 at 10:48:55PM +0100, Michael Krause wrote:
On 12/12/24 1:26 PM, Greg KH wrote:
On Tue, Dec 10, 2024 at 12:05:00AM +0100, Michael Krause wrote:
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote:
Michael Krause mk-debian@galax.is writes:
> On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote: > > Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF > > in smb2_reconnect_server()") which seems in fact to solve the issue. > > > > Michael, can you please post your backport here for review from Paulo > > and Steve? > > Of course, attached. > > Now I really hope I didn't screw it up :)
LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
It's corrupted somehow:
patching file fs/smb/client/connect.c patch: **** malformed patch at line 202: if (rc)
Can you resend it or attach it?
thanks,
greg k-h
Ugh, how embarrassing. I'm sorry, I "fixed" some minor whitespace issue directly in the patch and apparently did something wrong.
I redid the white space fix before diffing again and attach and inline the new version. The chunks are a bit alternated to the earlier version now unfortunately. This one applies..
Doesn't apply for me:
checking file fs/smb/client/connect.c Hunk #1 FAILED at 259. Hunk #2 FAILED at 1977. Hunk #3 FAILED at 2035. 3 out of 3 hunks FAILED checking file fs/smb/client/connect.c
Any ideas?
Hmm, that is strange. I just did the follwoing:
$ git branch 6.1.y-backport-smb-uaf-smb2_reconnect_server v6.1.119 $ git checkout 6.1.y-backport-smb-uaf-smb2_reconnect_server $ git am /tmp/backport-6.1-smb-client-fix-UAF-in-smb2_reconnect_server.v2.patch Applying: smb: client: fix UAF in smb2_reconnect_server() .git/rebase-apply/patch:102: space before tab in indent. spin_unlock(&ses->ses_lock); warning: 1 line adds whitespace errors.
The warning looks correct, there is a space before the indent here:
[...] 180 +^Ido_logoff = ses->ses_status == SES_GOOD && server->ops->logoff;$ 181 +^Ises->ses_status = SES_EXITING;$ 182 +^Itcon = ses->tcon_ipc;$ 183 +^Ises->tcon_ipc = NULL;$ 184 + ^Ispin_unlock(&ses->ses_lock);$ <--- space before the indent tab 185 +^Ispin_unlock(&cifs_tcp_ses_lock);$ 186 $ 187 -^Iif (ses->ses_status == SES_EXITING && server->ops->logoff) {$ [...]
Regards, Salvatore
On Fri, Dec 13, 2024 at 04:53:35PM +0100, Salvatore Bonaccorso wrote:
Hi Greg,
On Fri, Dec 13, 2024 at 03:33:31PM +0100, Greg KH wrote:
On Thu, Dec 12, 2024 at 10:48:55PM +0100, Michael Krause wrote:
On 12/12/24 1:26 PM, Greg KH wrote:
On Tue, Dec 10, 2024 at 12:05:00AM +0100, Michael Krause wrote:
On 12/3/24 3:45 PM, Salvatore Bonaccorso wrote:
Paulo,
On Tue, Dec 03, 2024 at 10:18:25AM -0300, Paulo Alcantara wrote: > Michael Krause mk-debian@galax.is writes: > > > On 11/30/24 10:21 AM, Salvatore Bonaccorso wrote: > > > Michael, did a manual backport of 24a9799aa8ef ("smb: client: fix UAF > > > in smb2_reconnect_server()") which seems in fact to solve the issue. > > > > > > Michael, can you please post your backport here for review from Paulo > > > and Steve? > > > > Of course, attached. > > > > Now I really hope I didn't screw it up :) > > LGTM. Thanks Michael for the backport.
Thanks a lot for the review. So to get it accepted it needs to be brough into the form which Greg can pick up. Michael can you do that and add your Signed-off line accordingly?
Happy to. Hope this is in the proper format:
It's corrupted somehow:
patching file fs/smb/client/connect.c patch: **** malformed patch at line 202: if (rc)
Can you resend it or attach it?
thanks,
greg k-h
Ugh, how embarrassing. I'm sorry, I "fixed" some minor whitespace issue directly in the patch and apparently did something wrong.
I redid the white space fix before diffing again and attach and inline the new version. The chunks are a bit alternated to the earlier version now unfortunately. This one applies..
Doesn't apply for me:
checking file fs/smb/client/connect.c Hunk #1 FAILED at 259. Hunk #2 FAILED at 1977. Hunk #3 FAILED at 2035. 3 out of 3 hunks FAILED checking file fs/smb/client/connect.c
Any ideas?
Hmm, that is strange. I just did the follwoing:
$ git branch 6.1.y-backport-smb-uaf-smb2_reconnect_server v6.1.119 $ git checkout 6.1.y-backport-smb-uaf-smb2_reconnect_server $ git am /tmp/backport-6.1-smb-client-fix-UAF-in-smb2_reconnect_server.v2.patch Applying: smb: client: fix UAF in smb2_reconnect_server() .git/rebase-apply/patch:102: space before tab in indent. spin_unlock(&ses->ses_lock); warning: 1 line adds whitespace errors.
The warning looks correct, there is a space before the indent here:
[...] 180 +^Ido_logoff = ses->ses_status == SES_GOOD && server->ops->logoff;$ 181 +^Ises->ses_status = SES_EXITING;$ 182 +^Itcon = ses->tcon_ipc;$ 183 +^Ises->tcon_ipc = NULL;$ 184 + ^Ispin_unlock(&ses->ses_lock);$ <--- space before the indent tab 185 +^Ispin_unlock(&cifs_tcp_ses_lock);$ 186 $ 187 -^Iif (ses->ses_status == SES_EXITING && server->ops->logoff) {$ [...]
Ok, this looks like it was a base64 issue on my side, with my tools, sorry about that. Now queued up!
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
gregkh＠linuxfoundation.org
-
Michael Krause -
Michael Krause -
Paulo Alcantara -
Salvatore Bonaccorso