From c746f8ae62703d2d6404b15e6dc80e370dcc473b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Wcis=C5=82o?= Date: Fri, 27 Mar 2026 00:27:31 +0100 Subject: [PATCH 1/4] cifs: fix potential deadlock in direct reclaim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jira VULN-169227 cve-pre CVE-2023-53751 commit-author Vincent Whitchurch commit cc391b694ff085f62f133e6b8f864d43a8e69dfd upstream-diff | Multiple differences across many files due to large discrepancies between upstream and LTS 8.6, HOWEVER, the commit was obtained in the exact same way as on upstream, namely: 1. locating all usages of `TCP_Server_Info::srv_mutex', 2. changing all `mutex_lock(&->srv_mutex)' to `cifs_server_lock()', 3. changing all `mutex_unlock(&->srv_mutex)' to `cifs_server_unlock()', 4. renaming `srv_mutex~' to `_srv_mutex' to make sure nothing was missed. In this regard the commit doesn't differ from the upstream. The srv_mutex is used during writeback so cifs should ensure that allocations done when that mutex is held are done with GFP_NOFS, to avoid having direct reclaim ending up waiting for the same mutex and causing a deadlock. This is detected by lockdep with the splat below: ====================================================== WARNING: possible circular locking dependency detected 5.18.0 #70 Not tainted ------------------------------------------------------ kswapd0/49 is trying to acquire lock: ffff8880195782e0 (&tcp_ses->srv_mutex){+.+.}-{3:3}, at: compound_send_recv but task is already holding lock: ffffffffa98e66c0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (fs_reclaim){+.+.}-{0:0}: fs_reclaim_acquire kmem_cache_alloc_trace __request_module crypto_alg_mod_lookup crypto_alloc_tfm_node crypto_alloc_shash cifs_alloc_hash smb311_crypto_shash_allocate smb311_update_preauth_hash compound_send_recv cifs_send_recv SMB2_negotiate smb2_negotiate cifs_negotiate_protocol cifs_get_smb_ses cifs_mount cifs_smb3_do_mount smb3_get_tree vfs_get_tree path_mount __x64_sys_mount do_syscall_64 entry_SYSCALL_64_after_hwframe -> #0 (&tcp_ses->srv_mutex){+.+.}-{3:3}: __lock_acquire lock_acquire __mutex_lock mutex_lock_nested compound_send_recv cifs_send_recv SMB2_write smb2_sync_write cifs_write cifs_writepage_locked cifs_writepage shrink_page_list shrink_lruvec shrink_node balance_pgdat kswapd kthread ret_from_fork other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(fs_reclaim); lock(&tcp_ses->srv_mutex); lock(fs_reclaim); lock(&tcp_ses->srv_mutex); *** DEADLOCK *** 1 lock held by kswapd0/49: #0: ffffffffa98e66c0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat stack backtrace: CPU: 2 PID: 49 Comm: kswapd0 Not tainted 5.18.0 #70 Call Trace: dump_stack_lvl dump_stack print_circular_bug.cold check_noncircular __lock_acquire lock_acquire __mutex_lock mutex_lock_nested compound_send_recv cifs_send_recv SMB2_write smb2_sync_write cifs_write cifs_writepage_locked cifs_writepage shrink_page_list shrink_lruvec shrink_node balance_pgdat kswapd kthread ret_from_fork Fix this by using the memalloc_nofs_save/restore APIs around the places where the srv_mutex is held. Do this in a wrapper function for the lock/unlock of the srv_mutex, and rename the srv_mutex to avoid missing call sites in the conversion. Note that there is another lockdep warning involving internal crypto locks, which was masked by this problem and is visible after this fix, see the discussion in this thread: https://lore.kernel.org/all/20220523123755.GA13668@axis.com/ Link: https://lore.kernel.org/r/CANT5p=rqcYfYMVHirqvdnnca4Mo+JQSw5Qu12v=kPfpk5yhhmg@mail.gmail.com/ Reported-by: Shyam Prasad N Suggested-by: Lars Persson Reviewed-by: Ronnie Sahlberg Reviewed-by: Enzo Matsumiya Signed-off-by: Vincent Whitchurch Signed-off-by: Steve French (cherry picked from commit cc391b694ff085f62f133e6b8f864d43a8e69dfd) Signed-off-by: Marcin Wcisło --- fs/cifs/cifs_swn.c | 4 ++-- fs/cifs/cifsencrypt.c | 8 ++++---- fs/cifs/cifsglob.h | 20 +++++++++++++++++++- fs/cifs/connect.c | 16 ++++++++-------- fs/cifs/sess.c | 6 +++--- fs/cifs/smb1ops.c | 6 +++--- fs/cifs/smb2pdu.c | 6 +++--- fs/cifs/smbdirect.c | 4 ++-- fs/cifs/transport.c | 32 ++++++++++++++++---------------- 9 files changed, 60 insertions(+), 42 deletions(-) diff --git a/fs/cifs/cifs_swn.c b/fs/cifs/cifs_swn.c index a172769c239f9..fe2fc8cc86e59 100644 --- a/fs/cifs/cifs_swn.c +++ b/fs/cifs/cifs_swn.c @@ -482,7 +482,7 @@ static int cifs_swn_store_swn_addr(const struct sockaddr_storage *new, static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *addr) { /* Store the reconnect address */ - mutex_lock(&tcon->ses->server->srv_mutex); + cifs_server_lock(tcon->ses->server); if (!cifs_sockaddr_equal(&tcon->ses->server->dstaddr, addr)) { int ret; @@ -520,7 +520,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a tcon->ses->server->tcpStatus = CifsNeedReconnect; spin_unlock(&GlobalMid_Lock); } - mutex_unlock(&tcon->ses->server->srv_mutex); + cifs_server_unlock(tcon->ses->server); return 0; } diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index 51d53e4bdf6b1..88d8437c02a70 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -245,9 +245,9 @@ int cifs_verify_signature(struct smb_rqst *rqst, cpu_to_le32(expected_sequence_number); cifs_pdu->Signature.Sequence.Reserved = 0; - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = cifs_calc_signature(rqst, server, what_we_think_sig_should_be); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc) return rc; @@ -716,7 +716,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) memcpy(ses->auth_key.response + baselen, tiblob, tilen); - mutex_lock(&ses->server->srv_mutex); + cifs_server_lock(ses->server); rc = cifs_alloc_hash("hmac(md5)", &ses->server->secmech.hmacmd5, @@ -768,7 +768,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__); unlock: - mutex_unlock(&ses->server->srv_mutex); + cifs_server_unlock(ses->server); setup_ntlmv2_rsp_ret: kfree(tiblob); diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index bf8ccff78b026..c62c7170a19ff 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -26,6 +26,7 @@ #include #include #include +#include #include "cifs_fs_sb.h" #include "cifsacl.h" #include @@ -603,7 +604,8 @@ struct TCP_Server_Info { unsigned int in_flight; /* number of requests on the wire to server */ unsigned int max_in_flight; /* max number of requests that were on wire */ spinlock_t req_lock; /* protect the two values above */ - struct mutex srv_mutex; + struct mutex _srv_mutex; + unsigned int nofs_flag; struct task_struct *tsk; char server_GUID[16]; __u16 sec_mode; @@ -695,6 +697,22 @@ struct TCP_Server_Info { #endif }; +static inline void cifs_server_lock(struct TCP_Server_Info *server) +{ + unsigned int nofs_flag = memalloc_nofs_save(); + + mutex_lock(&server->_srv_mutex); + server->nofs_flag = nofs_flag; +} + +static inline void cifs_server_unlock(struct TCP_Server_Info *server) +{ + unsigned int nofs_flag = server->nofs_flag; + + mutex_unlock(&server->_srv_mutex); + memalloc_nofs_restore(nofs_flag); +} + struct cifs_credits { unsigned int value; unsigned int instance; diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index b4525b39c9cfb..f74cb2bb31529 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -261,7 +261,7 @@ cifs_reconnect(struct TCP_Server_Info *server) /* do not want to be sending data on a socket we are freeing */ cifs_dbg(FYI, "%s: tearing down socket\n", __func__); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); if (server->ssocket) { cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n", server->ssocket->state, server->ssocket->flags); @@ -291,7 +291,7 @@ cifs_reconnect(struct TCP_Server_Info *server) mid_entry->mid_flags |= MID_DELETED; } spin_unlock(&GlobalMid_Lock); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__); list_for_each_safe(tmp, tmp2, &retry_list) { @@ -302,15 +302,15 @@ cifs_reconnect(struct TCP_Server_Info *server) } if (cifs_rdma_enabled(server)) { - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); smbd_destroy(server); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); } do { try_to_freeze(); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); #ifdef CONFIG_CIFS_SWN_UPCALL if (server->use_swn_dstaddr) { @@ -352,7 +352,7 @@ cifs_reconnect(struct TCP_Server_Info *server) rc = generic_ip_connect(server); if (rc) { cifs_dbg(FYI, "reconnect error %d\n", rc); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); msleep(3000); } else { atomic_inc(&tcpSesReconnectCount); @@ -364,7 +364,7 @@ cifs_reconnect(struct TCP_Server_Info *server) #ifdef CONFIG_CIFS_SWN_UPCALL server->use_swn_dstaddr = false; #endif - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); } } while (server->tcpStatus == CifsNeedReconnect); @@ -1332,7 +1332,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx) init_waitqueue_head(&tcp_ses->response_q); init_waitqueue_head(&tcp_ses->request_q); INIT_LIST_HEAD(&tcp_ses->pending_mid_q); - mutex_init(&tcp_ses->srv_mutex); + mutex_init(&tcp_ses->_srv_mutex); memcpy(tcp_ses->workstation_RFC1001_name, ctx->source_rfc1001_name, RFC1001_NAME_LEN_WITH_NULL); memcpy(tcp_ses->server_RFC1001_name, diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index 433eb07f52872..e982097515240 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -903,14 +903,14 @@ sess_establish_session(struct sess_data *sess_data) { struct cifs_ses *ses = sess_data->ses; - mutex_lock(&ses->server->srv_mutex); + cifs_server_lock(ses->server); if (!ses->server->session_estab) { if (ses->server->sign) { ses->server->session_key.response = kmemdup(ses->auth_key.response, ses->auth_key.len, GFP_KERNEL); if (!ses->server->session_key.response) { - mutex_unlock(&ses->server->srv_mutex); + cifs_server_unlock(ses->server); return -ENOMEM; } ses->server->session_key.len = @@ -919,7 +919,7 @@ sess_establish_session(struct sess_data *sess_data) ses->server->sequence_number = 0x2; ses->server->session_estab = true; } - mutex_unlock(&ses->server->srv_mutex); + cifs_server_unlock(ses->server); cifs_dbg(FYI, "CIFS session established successfully\n"); spin_lock(&GlobalMid_Lock); diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c index 017d0207befe6..e890d44990e00 100644 --- a/fs/cifs/smb1ops.c +++ b/fs/cifs/smb1ops.c @@ -49,10 +49,10 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_rqst *rqst, in_buf->WordCount = 0; put_bcc(0, in_buf); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = cifs_sign_smb(in_buf, server, &mid->sequence_number); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } @@ -66,7 +66,7 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_rqst *rqst, if (rc < 0) server->sequence_number--; - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "issued NT_CANCEL for mid %u, rc = %d\n", get_mid(in_buf), rc); diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 968e553d24025..1e1d00699bec0 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1264,13 +1264,13 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data) struct cifs_ses *ses = sess_data->ses; struct TCP_Server_Info *server = cifs_ses_server(ses); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); if (server->ops->generate_signingkey) { rc = server->ops->generate_signingkey(ses); if (rc) { cifs_dbg(FYI, "SMB3 session key generation failed\n"); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } } @@ -1278,7 +1278,7 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data) server->sequence_number = 0x2; server->session_estab = true; } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "SMB2/3 session established successfully\n"); /* keep existing ses state if binding */ diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index aadc745c3e35f..7eae9f1232d34 100644 --- a/fs/cifs/smbdirect.c +++ b/fs/cifs/smbdirect.c @@ -1381,9 +1381,9 @@ void smbd_destroy(struct TCP_Server_Info *server) log_rdma_event(INFO, "freeing mr list\n"); wake_up_interruptible_all(&info->wait_mr); while (atomic_read(&info->mr_used_count)) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); msleep(1000); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); } destroy_mr_list(info); diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 6644016846e05..25537ea1f3dcc 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -790,7 +790,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst, } else instance = exist_credits->instance; - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); /* * We can't use credits obtained from the previous session to send this @@ -798,14 +798,14 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst, * return -EAGAIN in such cases to let callers handle it. */ if (instance != server->reconnect_instance) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); add_credits_and_wake_if(server, &credits, optype); return -EAGAIN; } mid = server->ops->setup_async_request(server, rqst); if (IS_ERR(mid)) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); add_credits_and_wake_if(server, &credits, optype); return PTR_ERR(mid); } @@ -836,7 +836,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst, cifs_delete_mid(mid); } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc == 0) return 0; @@ -1078,7 +1078,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, * of smb data. */ - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); /* * All the parts of the compound chain belong obtained credits from the @@ -1088,7 +1088,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, * handle it. */ if (instance != server->reconnect_instance) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); for (j = 0; j < num_rqst; j++) add_credits(server, &credits[j], optype); return -EAGAIN; @@ -1100,7 +1100,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, revert_current_mid(server, i); for (j = 0; j < i; j++) cifs_delete_mid(midQ[j]); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); /* Update # of requests on wire to server */ for (j = 0; j < num_rqst; j++) @@ -1132,7 +1132,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, server->sequence_number -= 2; } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); /* * If sending failed for some reason or it is an oplock break that we @@ -1337,11 +1337,11 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses, and avoid races inside tcp sendmsg code that could cause corruption of smb data */ - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = allocate_mid(ses, in_buf, &midQ); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); /* Update # of requests on wire to server */ add_credits(server, &credits, 0); return rc; @@ -1349,7 +1349,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses, rc = cifs_sign_smb(in_buf, server, &midQ->sequence_number); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); goto out; } @@ -1363,7 +1363,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses, if (rc < 0) server->sequence_number -= 2; - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc < 0) goto out; @@ -1479,18 +1479,18 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon, and avoid races inside tcp sendmsg code that could cause corruption of smb data */ - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = allocate_mid(ses, in_buf, &midQ); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } rc = cifs_sign_smb(in_buf, server, &midQ->sequence_number); if (rc) { cifs_delete_mid(midQ); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } @@ -1503,7 +1503,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon, if (rc < 0) server->sequence_number -= 2; - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc < 0) { cifs_delete_mid(midQ); From 601cd8f6b397cfb8698f40085df609621e309e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Wcis=C5=82o?= Date: Fri, 27 Mar 2026 00:42:53 +0100 Subject: [PATCH 2/4] cifs: fix race in assemble_neg_contexts() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jira VULN-169227 cve CVE-2023-53751 commit-author Paulo Alcantara commit 775e44d6d86dca400d614cbda5dab4def4951fe7 upstream-diff Ignored the introduction of `pserver' variable, as well as the usage of `hostname' local, as they were only needed in the upstream because of the dual source of the server hostname, introduced in the non-backported commit 9de74996a739bf0b7b5d8c260bd207ad6007442b ("smb3: use netname when available on secondary channels") Serialise access of TCP_Server_Info::hostname in assemble_neg_contexts() by holding the server's mutex otherwise it might end up accessing an already-freed hostname pointer from cifs_reconnect() or cifs_resolve_server(). Signed-off-by: Paulo Alcantara (SUSE) Reviewed-by: Enzo Matsumiya Signed-off-by: Steve French (cherry picked from commit 775e44d6d86dca400d614cbda5dab4def4951fe7) Signed-off-by: Marcin Wcisło --- fs/cifs/smb2pdu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 1e1d00699bec0..2eca48b9b6fc1 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -549,8 +549,10 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, } else req->NegotiateContextCount = cpu_to_le16(4); + cifs_server_lock(server); ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt, server->hostname); + cifs_server_unlock(server); *total_len += ctxt_len; pneg_ctxt += ctxt_len; From 9d7bba34b26e8eabc48dbcf8bdbddf97630b2b24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Wcis=C5=82o?= Date: Fri, 27 Mar 2026 23:34:59 +0100 Subject: [PATCH 3/4] cifs: protect access of TCP_Server_Info::{dstaddr,hostname} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jira VULN-169227 cve CVE-2023-53751 commit-author Paulo Alcantara commit 39a154fc2d172a3a5865e5a9fa2a2983eb7a99ac upstream-diff Manually obtained commit. Upstream fixes appear in multiple places, but they all cover only the `cifs_tree_connect()' function (the CONFIG_CIFS_DFS_UPCALL=y variant) and its call tree. Same goes for the backported version, taking into account the lack of a major function rewrite in c88f7dcd6d6429197fc2fd87b54a894ffcd48e8e ("cifs: support nested dfs links over reconnect"). Upstream call tree at the moment of the fix and the changes at each level: cifs_tree_connect() Put `cifs_server_{lock/unlock}(server)' around `scnprintf(..., server->hostname);' call. tree_connect_dfs_target() No changes. __tree_connect_dfs_target() Removed `extract_unc_hostname(server->hostname, ...)' call, used full `server->hostname' instead of its substring `tcp_host'. target_share_matches_server() Put `cifs_server_{lock/unlock}(server)' around `server->hostname' accesses in conditional expression and debug message composition before the `match_target_ip()' call. match_target_ip() Put `spin_{lock/unlock}(&server->srv_lock)' around `cifs_match_ipaddr(...server->dstaddr, ....)' call. Backported version call tree: cifs_tree_connect() - Put `cifs_server_{lock/unlock}(server)' around `scnprintf(..., server->hostname)' call - like in the upstream. - Put `cifs_server_{lock/unlock}(server)' around `server->hostname' (and its substring `tcp_host') access in conditional expression and debug message composition before the `match_target_ip()' call - like in the upstream. This required moving the `extract_unc_hostname()' call inside the loop. Unlike in the upstream it was not removed entirely, opting for functional equivalence between the patched and non-patched version instead of strictly preserving backported commit's changes; perhaps at the time of the upstream fix the equality `server->hostname' = `tcp_host' held true, allowing for the reduction of `extract_unc_hostname()' call, but that was not certain for the ciqlts8_6 version. If it wasn't true then the commit mixed a bugfix with a functional change which was simply filtered out here. Moving `extract_unc_hostname()' call inside the loop did not create any algorithmic inconsistencies which weren't already present (if any), merely preventing them from ending in bad memory access. In the upstream the value of `server->hostname' isn't saved to remain constant for all loop iterations in the `__tree_connect_dfs_target()' function either. A ngeligible performance penalty associated with the calculation redundancy was accepted given the simplicity of the fix it allowed. match_target_ip() Put `spin_{lock/unlock}(&cifs_tcp_ses_lock)' around the `cifs_match_ipaddr(...server->dstaddr, ...)' call - like in the upstream, except for the `cifs_tcp_ses_lock' instead of `server->srv_lock'. The latter was introduced in the non-backported commit d7d7a66aacd6fd8ca57baf08a7bac5421282f6f8 ("cifs: avoid use of global locks for high contention data"). The same pattern of protecting `server->dstaddr' with `cifs_tcp_ses_lock' can be observed in ciqlts8_6 in the `reconn_set_ipaddr_from_hostname()' function. Use the appropriate locks to protect access of hostname and dstaddr fields in cifs_tree_connect() as they might get changed by other tasks. Signed-off-by: Paulo Alcantara (SUSE) Reviewed-by: Enzo Matsumiya Signed-off-by: Steve French (cherry picked from commit 39a154fc2d172a3a5865e5a9fa2a2983eb7a99ac) Signed-off-by: Marcin Wcisło --- fs/cifs/connect.c | 10 +++++++--- fs/cifs/misc.c | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index f74cb2bb31529..7ae8a903baf24 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -4081,7 +4081,9 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru if (!tcon->dfs_path) { if (tcon->ipc) { + cifs_server_lock(server); scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$", server->hostname); + cifs_server_unlock(server); rc = ops->tree_connect(xid, tcon->ses, tree, tcon, nlsc); } else { rc = ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc); @@ -4095,8 +4097,6 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru isroot = ref.server_type == DFS_TYPE_ROOT; free_dfs_info_param(&ref); - extract_unc_hostname(server->hostname, &tcp_host, &tcp_host_len); - for (it = dfs_cache_get_tgt_iterator(&tl); it; it = dfs_cache_get_next_tgt(&tl, it)) { bool target_match; @@ -4114,10 +4114,13 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru extract_unc_hostname(share, &dfs_host, &dfs_host_len); + cifs_server_lock(server); + extract_unc_hostname(server->hostname, &tcp_host, &tcp_host_len); if (dfs_host_len != tcp_host_len || strncasecmp(dfs_host, tcp_host, dfs_host_len) != 0) { cifs_dbg(FYI, "%s: %.*s doesn't match %.*s\n", __func__, (int)dfs_host_len, dfs_host, (int)tcp_host_len, tcp_host); + cifs_server_unlock(server); rc = match_target_ip(server, dfs_host, dfs_host_len, &target_match); if (rc) { @@ -4129,7 +4132,8 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru cifs_dbg(FYI, "%s: skipping target\n", __func__); continue; } - } + } else + cifs_server_unlock(server); if (tcon->ipc) { scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$", share); diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 0d46f56653c29..ecf9e0f8e297a 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -1123,8 +1123,10 @@ int match_target_ip(struct TCP_Server_Info *server, goto out; } + spin_lock(&cifs_tcp_ses_lock); *result = cifs_match_ipaddr((struct sockaddr *)&server->dstaddr, &tipaddr); + spin_unlock(&cifs_tcp_ses_lock); cifs_dbg(FYI, "%s: ip addresses match: %u\n", __func__, *result); rc = 0; From ac27ef5178581db10130496965c8929622931c7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Wcis=C5=82o?= Date: Mon, 23 Mar 2026 00:26:48 +0100 Subject: [PATCH 4/4] cifs: fix potential use-after-free bugs in TCP_Server_Info::hostname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jira VULN-169227 cve CVE-2023-53751 commit-author Paulo Alcantara commit 90c49fce1c43e1cc152695e20363ff5087897c09 upstream-diff | fs/cifs/cifsglob.h Added `srv_lock' to the `TCP_Server_Info' struct as it was done in the non-backported commit d7d7a66aacd6fd8ca57baf08a7bac5421282f6f8 ("cifs: avoid use of global locks for high contention data"). fs/cifs/cifs_debug.c Ignored changes in `cifs_debug_data_proc_show()' because the code using `hostname' was only added in the non-backported commit 40f077a02bf9d70719128d2a807e28a3503711eb ("cifs: clarify hostname vs ip address in /proc/fs/cifs/DebugData"). fs/cifs/connect.c - Changed the `reconn_set_next_dfs_target()' function instead of `__reconnect_target_unlocked()' which doesn't exist in LTS 8.6. The `__reconnect_target_unlocked()' is where the `hostname' is "updated once or many times during reconnect" (see original commit message). The "reconnect" refers to the function `cifs_reconnect()', which calls `__reconnect_target_unlocked()' on a third level down the call tree. Relative to LTS 8.6 the `cifs_reconnect()' underwent major rewrites in the commits bbcce368044572d0802c3bbb8ef3fe98f581d803 ("cifs: split out dfs code from cifs_reconnect()") and c88f7dcd6d6429197fc2fd87b54a894ffcd48e8e ("cifs: support nested dfs links over reconnect"). In LTS 8.6 the updating of `hostname' can be tracked down to the `reconn_set_next_dfs_target()' function called by `cifs_reconnect()'. - Added initialization of `TCP_Server_Info::srv_lock' in `cifs_get_tcp_session()' as it's done in d7d7a66aa. - In `match_server()' changed the lockdep assertion from holding `server->srv_lock' to holding `cifs_tcp_ses_lock', because this is the actual invariant for the LTS 8.6 version. - In `match_server()' sandwiched the `strcasecmp(server->hostname, ...)' call in the lock/unlock pair of `server->srv_lock', because in LTS 8.6 the `match_server()' is not called with the `server->srv_lock' held and yet this is the lock chosen to protect the `server->hostname' field. fs/cifs/sess.c Ignored changes in `cifs_try_adding_channels()' because the code using `hostname' was only added in the non-backported commit 9c2dc11df50d1c8537075ff6b98472198e24438e ("smb3: do not attempt multichannel to server which does not support it"). As a result no changes to the file have been made. TCP_Server_Info::hostname may be updated once or many times during reconnect, so protect its access outside reconnect path as well and then prevent any potential use-after-free bugs. Cc: stable@vger.kernel.org Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French (cherry picked from commit 90c49fce1c43e1cc152695e20363ff5087897c09) Signed-off-by: Marcin Wcisło --- fs/cifs/cifs_debug.c | 5 ++++- fs/cifs/cifs_debug.h | 12 ++++++------ fs/cifs/cifsglob.h | 1 + fs/cifs/connect.c | 17 +++++++++++++---- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c index eb4f3a982dfab..78a3c3462c7f5 100644 --- a/fs/cifs/cifs_debug.c +++ b/fs/cifs/cifs_debug.c @@ -609,10 +609,13 @@ static int cifs_stats_proc_show(struct seq_file *m, void *v) server->fastest_cmd[j], server->slowest_cmd[j]); for (j = 0; j < NUMBER_OF_SMB2_COMMANDS; j++) - if (atomic_read(&server->smb2slowcmd[j])) + if (atomic_read(&server->smb2slowcmd[j])) { + spin_lock(&server->srv_lock); seq_printf(m, " %d slow responses from %s for command %d\n", atomic_read(&server->smb2slowcmd[j]), server->hostname, j); + spin_unlock(&server->srv_lock); + } #endif /* STATS2 */ list_for_each(tmp2, &server->smb_ses_list) { ses = list_entry(tmp2, struct cifs_ses, diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h index 4da811d5f7c4b..d2daab8b79b79 100644 --- a/fs/cifs/cifs_debug.h +++ b/fs/cifs/cifs_debug.h @@ -95,19 +95,19 @@ do { \ #define cifs_server_dbg_func(ratefunc, type, fmt, ...) \ do { \ - const char *sn = ""; \ - if (server && server->hostname) \ - sn = server->hostname; \ + spin_lock(&server->srv_lock); \ if ((type) & FYI && cifsFYI & CIFS_INFO) { \ pr_debug_ ## ratefunc("%s: \\\\%s " fmt, \ - __FILE__, sn, ##__VA_ARGS__); \ + __FILE__, server->hostname, \ + ##__VA_ARGS__); \ } else if ((type) & VFS) { \ pr_err_ ## ratefunc("VFS: \\\\%s " fmt, \ - sn, ##__VA_ARGS__); \ + server->hostname, ##__VA_ARGS__); \ } else if ((type) & NOISY && (NOISY != 0)) { \ pr_debug_ ## ratefunc("\\\\%s " fmt, \ - sn, ##__VA_ARGS__); \ + server->hostname, ##__VA_ARGS__); \ } \ + spin_unlock(&server->srv_lock); \ } while (0) #define cifs_server_dbg(type, fmt, ...) \ diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index c62c7170a19ff..33f08aed817fe 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -580,6 +580,7 @@ inc_rfc1001_len(void *buf, int count) struct TCP_Server_Info { struct list_head tcp_ses_list; struct list_head smb_ses_list; + spinlock_t srv_lock; /* protect anything here that is not protected */ int srv_count; /* reference counter */ /* 15 character server name + 0x20 16th byte indicating type = srv */ char server_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL]; diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 7ae8a903baf24..7ef80ad3c237d 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -147,9 +147,11 @@ static void reconn_set_next_dfs_target(struct TCP_Server_Info *server, name = dfs_cache_get_tgt_name(*tgt_it); + spin_lock(&server->srv_lock); kfree(server->hostname); server->hostname = extract_hostname(name); + spin_unlock(&server->srv_lock); if (IS_ERR(server->hostname)) { cifs_dbg(FYI, "%s: failed to extract hostname from target: %ld\n", @@ -418,9 +420,7 @@ cifs_echo_request(struct work_struct *work) goto requeue_echo; rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS; - if (rc) - cifs_dbg(FYI, "Unable to send echo request to server: %s\n", - server->hostname); + cifs_server_dbg(FYI, "send echo request: rc = %d\n", rc); #ifdef CONFIG_CIFS_SWN_UPCALL /* Check witness registrations */ @@ -1177,6 +1177,8 @@ static int match_server(struct TCP_Server_Info *server, struct smb3_fs_context * { struct sockaddr *addr = (struct sockaddr *)&ctx->dstaddr; + lockdep_assert_held(&cifs_tcp_ses_lock); + if (ctx->nosharesock) return 0; @@ -1194,8 +1196,12 @@ static int match_server(struct TCP_Server_Info *server, struct smb3_fs_context * if (!net_eq(cifs_net_ns(server), current->nsproxy->net_ns)) return 0; - if (strcasecmp(server->hostname, ctx->server_hostname)) + spin_lock(&server->srv_lock); + if (strcasecmp(server->hostname, ctx->server_hostname)) { + spin_unlock(&server->srv_lock); return 0; + } + spin_unlock(&server->srv_lock); if (!match_address(server, addr, (struct sockaddr *)&ctx->srcaddr)) @@ -1343,6 +1349,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx) tcp_ses->lstrp = jiffies; tcp_ses->compress_algorithm = cpu_to_le16(ctx->compression); spin_lock_init(&tcp_ses->req_lock); + spin_lock_init(&tcp_ses->srv_lock); INIT_LIST_HEAD(&tcp_ses->tcp_ses_list); INIT_LIST_HEAD(&tcp_ses->smb_ses_list); INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request); @@ -1512,7 +1519,9 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb3_fs_context *ctx) if (tcon == NULL) return -ENOMEM; + spin_lock(&server->srv_lock); scnprintf(unc, sizeof(unc), "\\\\%s\\IPC$", server->hostname); + spin_unlock(&server->srv_lock); xid = get_xid(); tcon->ses = ses;