mainline inclusion from mainline-v6.17-rc2 commit e3835731e169a48a2c73018d135b5c08c39ea61d category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/ID5RU9 CVE: NA Reference: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commi... ------------------------------- This is step 4/4 of a patch series to fix mid_q_entry memory leaks caused by race conditions in callback execution. In compound_send_recv(), when wait_for_response() is interrupted by signals, the code attempts to cancel pending requests by changing their callbacks to cifs_cancelled_callback. However, there's a race condition between signal interruption and network response processing that causes both mid_q_entry and server buffer leaks: ``` User foreground process cifsd cifs_readdir open_cached_dir cifs_send_recv compound_send_recv smb2_setup_request smb2_mid_entry_alloc smb2_get_mid_entry smb2_mid_entry_alloc mempool_alloc // alloc mid kref_init(&temp->refcount); // refcount = 1 mid[0]->callback = cifs_compound_callback; mid[1]->callback = cifs_compound_last_callback; smb_send_rqst rc = wait_for_response wait_event_state TASK_KILLABLE cifs_demultiplex_thread allocate_buffers server->bigbuf = cifs_buf_get() standard_receive3 ->find_mid() smb2_find_mid __smb2_find_mid kref_get(&mid->refcount) // +1 cifs_handle_standard handle_mid /* bigbuf will also leak */ mid->resp_buf = server->bigbuf server->bigbuf = NULL; dequeue_mid /* in for loop */ mids[0]->callback cifs_compound_callback /* Signal interrupts wait: rc = -ERESTARTSYS */ /* if (... || midQ[i]->mid_state == MID_RESPONSE_RECEIVED) *? midQ[0]->callback = cifs_cancelled_callback; cancelled_mid[i] = true; /* The change comes too late */ mid->mid_state = MID_RESPONSE_READY release_mid // -1 /* cancelled_mid[i] == true causes mid won't be released in compound_send_recv cleanup */ /* cifs_cancelled_callback won't executed to release mid */ ``` The root cause is that there's a race between callback assignment and execution. Fix this by introducing per-mid locking: - Add spinlock_t mid_lock to struct mid_q_entry - Add mid_execute_callback() for atomic callback execution - Use mid_lock in cancellation paths to ensure atomicity This ensures that either the original callback or the cancellation callback executes atomically, preventing reference count leaks when requests are interrupted by signals. Link: https://bugzilla.kernel.org/show_bug.cgi?id=220404 Fixes: ee258d79159a ("CIFS: Move credit processing to mid callbacks for SMB3") Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Signed-off-by: Steve French <stfrench@microsoft.com> Conflicts: fs/smb/client/cifsglob.h fs/smb/client/connect.c fs/smb/client/smb2ops.c fs/smb/client/transport.c fs/smb/client/cifstransport.c [Conflict with code context.] Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> --- fs/smb/client/cifsglob.h | 20 ++++++++++++++++++++ fs/smb/client/connect.c | 6 +++--- fs/smb/client/smb2ops.c | 4 ++-- fs/smb/client/smb2transport.c | 1 + fs/smb/client/transport.c | 26 ++++++++++++-------------- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 8dbc93bdd013..1e6d0c87346c 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -1729,10 +1729,11 @@ struct mid_q_entry { unsigned int resp_buf_size; int mid_state; /* wish this were enum but can not pass to wait_event */ unsigned int mid_flags; __le16 command; /* smb command code */ unsigned int optype; /* operation type */ + spinlock_t mid_lock; bool large_buf:1; /* if valid response, is pointer to large buf */ bool multiRsp:1; /* multiple trans2 responses for one request */ bool multiEnd:1; /* both received */ bool decrypted:1; /* decrypted entry */ }; @@ -2030,10 +2031,13 @@ require use of the stronger protocol */ * cifsFileInfo->fh_mutex cifsFileInfo cifs_new_fileinfo * cifsFileInfo->file_info_lock cifsFileInfo->count cifs_new_fileinfo * ->invalidHandle initiate_cifs_search * ->oplock_break_cancelled * cifs_aio_ctx->aio_mutex cifs_aio_ctx cifs_aio_ctx_alloc + * mid_q_entry->mid_lock mid_q_entry->callback alloc_mid + * smb2_mid_entry_alloc + * (Any fields of mid_q_entry that will need protection) ****************************************************************************/ #ifdef DECLARE_GLOBALS_HERE #define GLOBAL_EXTERN #else @@ -2345,6 +2349,22 @@ static inline bool cifs_ses_exiting(struct cifs_ses *ses) ret = ses->ses_status == SES_EXITING; spin_unlock(&ses->ses_lock); return ret; } +/* + * Execute mid callback atomically - ensures callback runs exactly once + * and prevents sleeping in atomic context. + */ +static inline void mid_execute_callback(struct mid_q_entry *mid) +{ + void (*callback)(struct mid_q_entry *mid); + + spin_lock(&mid->mid_lock); + callback = mid->callback; + mid->callback = NULL; /* Mark as executed, */ + spin_unlock(&mid->mid_lock); + + if (callback) + callback(mid); +} #endif /* _CIFS_GLOB_H */ diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 4c730e1fde20..23e0b902239a 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -339,11 +339,11 @@ cifs_abort_connection(struct TCP_Server_Info *server) cifs_server_unlock(server); cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__); list_for_each_entry_safe(mid, nmid, &retry_list, qhead) { list_del_init(&mid->qhead); - mid->callback(mid); + mid_execute_callback(mid); release_mid(mid); } if (cifs_rdma_enabled(server)) { cifs_server_lock(server); @@ -1028,11 +1028,11 @@ clean_demultiplex_info(struct TCP_Server_Info *server) /* now walk dispose list and issue callbacks */ list_for_each_safe(tmp, tmp2, &dispose_list) { mid_entry = list_entry(tmp, struct mid_q_entry, qhead); cifs_dbg(FYI, "Callback mid %llu\n", mid_entry->mid); list_del_init(&mid_entry->qhead); - mid_entry->callback(mid_entry); + mid_execute_callback(mid_entry); release_mid(mid_entry); } /* 1/8th of sec is more than enough time for them to exit */ msleep(125); } @@ -1305,11 +1305,11 @@ cifs_demultiplex_thread(void *p) "Share deleted. Reconnect needed"); } } if (!mids[i]->multiRsp || mids[i]->multiEnd) - mids[i]->callback(mids[i]); + mid_execute_callback(mids[i]); release_mid(mids[i]); } else if (server->ops->is_oplock_break && server->ops->is_oplock_break(bufs[i], server)) { diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index ce1fb2f3e095..8c76d6ce5a25 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -4727,19 +4727,19 @@ static void smb2_decrypt_offload(struct work_struct *work) #endif if (dw->server->ops->is_network_name_deleted) dw->server->ops->is_network_name_deleted(dw->buf, dw->server); - mid->callback(mid); + mid_execute_callback(mid); } else { spin_lock(&dw->server->srv_lock); if (dw->server->tcpStatus == CifsNeedReconnect) { spin_lock(&dw->server->mid_lock); mid->mid_state = MID_RETRY_NEEDED; spin_unlock(&dw->server->mid_lock); spin_unlock(&dw->server->srv_lock); - mid->callback(mid); + mid_execute_callback(mid); } else { spin_lock(&dw->server->mid_lock); mid->mid_state = MID_REQUEST_SUBMITTED; mid->mid_flags &= ~(MID_DELETED); list_add_tail(&mid->qhead, diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c index 4a43802375b3..7f21e6ae8c5b 100644 --- a/fs/smb/client/smb2transport.c +++ b/fs/smb/client/smb2transport.c @@ -769,10 +769,11 @@ smb2_mid_entry_alloc(const struct smb2_hdr *shdr, } temp = mempool_alloc(cifs_mid_poolp, GFP_NOFS); memset(temp, 0, sizeof(struct mid_q_entry)); kref_init(&temp->refcount); + spin_lock_init(&temp->mid_lock); temp->mid = le64_to_cpu(shdr->MessageId); temp->credits = credits > 0 ? credits : 1; temp->pid = current->pid; temp->command = shdr->Command; /* Always LE */ temp->when_alloc = jiffies; diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c index ddf1a3aafee5..8f045db44319 100644 --- a/fs/smb/client/transport.c +++ b/fs/smb/client/transport.c @@ -51,10 +51,11 @@ alloc_mid(const struct smb_hdr *smb_buffer, struct TCP_Server_Info *server) } temp = mempool_alloc(cifs_mid_poolp, GFP_NOFS); memset(temp, 0, sizeof(struct mid_q_entry)); kref_init(&temp->refcount); + spin_lock_init(&temp->mid_lock); temp->mid = get_mid(smb_buffer); temp->pid = current->pid; temp->command = cpu_to_le16(smb_buffer->Command); cifs_dbg(FYI, "For smb_command %d\n", smb_buffer->Command); /* easier to use jiffies */ @@ -1218,19 +1219,18 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, if (rc != 0) { for (; i < num_rqst; i++) { cifs_server_dbg(FYI, "Cancelling wait for mid %llu cmd: %d\n", midQ[i]->mid, le16_to_cpu(midQ[i]->command)); send_cancel(server, &rqst[i], midQ[i]); - spin_lock(&server->mid_lock); + spin_lock(&midQ[i]->mid_lock); midQ[i]->mid_flags |= MID_WAIT_CANCELLED; - if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED || - midQ[i]->mid_state == MID_RESPONSE_RECEIVED) { + if (midQ[i]->callback) { midQ[i]->callback = cifs_cancelled_callback; cancelled_mid[i] = true; credits[i].value = 0; } - spin_unlock(&server->mid_lock); + spin_unlock(&midQ[i]->mid_lock); } } for (i = 0; i < num_rqst; i++) { if (rc < 0) @@ -1428,20 +1428,19 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses, goto out; rc = wait_for_response(server, midQ); if (rc != 0) { send_cancel(server, &rqst, midQ); - spin_lock(&server->mid_lock); - if (midQ->mid_state == MID_REQUEST_SUBMITTED || - midQ->mid_state == MID_RESPONSE_RECEIVED) { + spin_lock(&midQ->mid_lock); + if (midQ->callback) { /* no longer considered to be "in-flight" */ midQ->callback = release_mid; - spin_unlock(&server->mid_lock); + spin_unlock(&midQ->mid_lock); add_credits(server, &credits, 0); return rc; } - spin_unlock(&server->mid_lock); + spin_unlock(&midQ->mid_lock); } rc = cifs_sync_mid_result(midQ, server); if (rc != 0) { add_credits(server, &credits, 0); @@ -1610,19 +1609,18 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon, } rc = wait_for_response(server, midQ); if (rc) { send_cancel(server, &rqst, midQ); - spin_lock(&server->mid_lock); - if (midQ->mid_state == MID_REQUEST_SUBMITTED || - midQ->mid_state == MID_RESPONSE_RECEIVED) { + spin_lock(&midQ->mid_lock); + if (midQ->callback) { /* no longer considered to be "in-flight" */ midQ->callback = release_mid; - spin_unlock(&server->mid_lock); + spin_unlock(&midQ->mid_lock); return rc; } - spin_unlock(&server->mid_lock); + spin_unlock(&midQ->mid_lock); } /* We got the response - restart system call. */ rstart = 1; spin_lock(&server->srv_lock); -- 2.34.3