Skip to content

Commit 9502dd5

Browse files
pcacjrSteve French
authored and
Steve French
committed
smb: client: fix UAF in decryption with multichannel
After commit f7025d8 ("smb: client: allocate crypto only for primary server") and commit b0abcd6 ("smb: client: fix UAF in async decryption"), the channels started reusing AEAD TFM from primary channel to perform synchronous decryption, but that can't done as there could be multiple cifsd threads (one per channel) simultaneously accessing it to perform decryption. This fixes the following KASAN splat when running fstest generic/249 with 'vers=3.1.1,multichannel,max_channels=4,seal' against Windows Server 2022: BUG: KASAN: slab-use-after-free in gf128mul_4k_lle+0xba/0x110 Read of size 8 at addr ffff8881046c18a0 by task cifsd/986 CPU: 3 UID: 0 PID: 986 Comm: cifsd Not tainted 6.15.0-rc1 #1 PREEMPT(voluntary) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-3.fc41 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x5d/0x80 print_report+0x156/0x528 ? gf128mul_4k_lle+0xba/0x110 ? __virt_addr_valid+0x145/0x300 ? __phys_addr+0x46/0x90 ? gf128mul_4k_lle+0xba/0x110 kasan_report+0xdf/0x1a0 ? gf128mul_4k_lle+0xba/0x110 gf128mul_4k_lle+0xba/0x110 ghash_update+0x189/0x210 shash_ahash_update+0x295/0x370 ? __pfx_shash_ahash_update+0x10/0x10 ? __pfx_shash_ahash_update+0x10/0x10 ? __pfx_extract_iter_to_sg+0x10/0x10 ? ___kmalloc_large_node+0x10e/0x180 ? __asan_memset+0x23/0x50 crypto_ahash_update+0x3c/0xc0 gcm_hash_assoc_remain_continue+0x93/0xc0 crypt_message+0xe09/0xec0 [cifs] ? __pfx_crypt_message+0x10/0x10 [cifs] ? _raw_spin_unlock+0x23/0x40 ? __pfx_cifs_readv_from_socket+0x10/0x10 [cifs] decrypt_raw_data+0x229/0x380 [cifs] ? __pfx_decrypt_raw_data+0x10/0x10 [cifs] ? __pfx_cifs_read_iter_from_socket+0x10/0x10 [cifs] smb3_receive_transform+0x837/0xc80 [cifs] ? __pfx_smb3_receive_transform+0x10/0x10 [cifs] ? __pfx___might_resched+0x10/0x10 ? __pfx_smb3_is_transform_hdr+0x10/0x10 [cifs] cifs_demultiplex_thread+0x692/0x1570 [cifs] ? __pfx_cifs_demultiplex_thread+0x10/0x10 [cifs] ? rcu_is_watching+0x20/0x50 ? rcu_lockdep_current_cpu_online+0x62/0xb0 ? find_held_lock+0x32/0x90 ? kvm_sched_clock_read+0x11/0x20 ? local_clock_noinstr+0xd/0xd0 ? trace_irq_enable.constprop.0+0xa8/0xe0 ? __pfx_cifs_demultiplex_thread+0x10/0x10 [cifs] kthread+0x1fe/0x380 ? kthread+0x10f/0x380 ? __pfx_kthread+0x10/0x10 ? local_clock_noinstr+0xd/0xd0 ? ret_from_fork+0x1b/0x60 ? local_clock+0x15/0x30 ? lock_release+0x29b/0x390 ? rcu_is_watching+0x20/0x50 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x31/0x60 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> Tested-by: David Howells <[email protected]> Reported-by: Steve French <[email protected]> Closes: https://lore.kernel.org/r/CAH2r5mu6Yc0-RJXM3kFyBYUB09XmXBrNodOiCVR4EDrmxq5Szg@mail.gmail.com Fixes: f7025d8 ("smb: client: allocate crypto only for primary server") Fixes: b0abcd6 ("smb: client: fix UAF in async decryption") Signed-off-by: Paulo Alcantara (Red Hat) <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent c7efac7 commit 9502dd5

File tree

3 files changed

+10
-23
lines changed

3 files changed

+10
-23
lines changed

fs/smb/client/cifsencrypt.c

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -704,18 +704,12 @@ cifs_crypto_secmech_release(struct TCP_Server_Info *server)
704704
cifs_free_hash(&server->secmech.md5);
705705
cifs_free_hash(&server->secmech.sha512);
706706

707-
if (!SERVER_IS_CHAN(server)) {
708-
if (server->secmech.enc) {
709-
crypto_free_aead(server->secmech.enc);
710-
server->secmech.enc = NULL;
711-
}
712-
713-
if (server->secmech.dec) {
714-
crypto_free_aead(server->secmech.dec);
715-
server->secmech.dec = NULL;
716-
}
717-
} else {
707+
if (server->secmech.enc) {
708+
crypto_free_aead(server->secmech.enc);
718709
server->secmech.enc = NULL;
710+
}
711+
if (server->secmech.dec) {
712+
crypto_free_aead(server->secmech.dec);
719713
server->secmech.dec = NULL;
720714
}
721715
}

fs/smb/client/smb2ops.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4555,9 +4555,9 @@ decrypt_raw_data(struct TCP_Server_Info *server, char *buf,
45554555
return rc;
45564556
}
45574557
} else {
4558-
if (unlikely(!server->secmech.dec))
4559-
return -EIO;
4560-
4558+
rc = smb3_crypto_aead_allocate(server);
4559+
if (unlikely(rc))
4560+
return rc;
45614561
tfm = server->secmech.dec;
45624562
}
45634563

fs/smb/client/smb2pdu.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,15 +1252,8 @@ SMB2_negotiate(const unsigned int xid,
12521252
cifs_server_dbg(VFS, "Missing expected negotiate contexts\n");
12531253
}
12541254

1255-
if (server->cipher_type && !rc) {
1256-
if (!SERVER_IS_CHAN(server)) {
1257-
rc = smb3_crypto_aead_allocate(server);
1258-
} else {
1259-
/* For channels, just reuse the primary server crypto secmech. */
1260-
server->secmech.enc = server->primary_server->secmech.enc;
1261-
server->secmech.dec = server->primary_server->secmech.dec;
1262-
}
1263-
}
1255+
if (server->cipher_type && !rc)
1256+
rc = smb3_crypto_aead_allocate(server);
12641257
neg_exit:
12651258
free_rsp_buf(resp_buftype, rsp);
12661259
return rc;

0 commit comments

Comments
 (0)