smb: client: fix mid_q_entry memleak leak with per-mid locking

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>

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index e6830ab3a546c099862b510ab5c281bfec210fae..1e64a4fb6af037636e55a004522258c35dacbd22 100644 (file)
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1732,6 +1732,7 @@ struct mid_q_entry {
        int mid_rc;             /* rc for MID_RC */
        __le16 command;         /* smb command code */
        unsigned int optype;    /* operation type */
+       spinlock_t mid_lock;
        bool wait_cancelled:1;  /* Cancelled while waiting for response */
        bool deleted_from_q:1;  /* Whether Mid has been dequeued frem pending_mid_q */
        bool large_buf:1;       /* if valid response, is pointer to large buf */
@@ -2036,6 +2037,9 @@ require use of the stronger protocol */
  * cifsFileInfo->file_info_lock        cifsFileInfo->count             cifs_new_fileinfo
  *                             ->invalidHandle                 initiate_cifs_search
  *                             ->oplock_break_cancelled
+ * 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
@@ -2375,6 +2379,23 @@ static inline bool cifs_netbios_name(const char *name, size_t namelen)
        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);
+}
+
 #define CIFS_REPARSE_SUPPORT(tcon) \
        ((tcon)->posix_extensions || \
         (le32_to_cpu((tcon)->fsAttrInfo.Attributes) & \

diff --git a/fs/smb/client/cifstransport.c b/fs/smb/client/cifstransport.c
index 352dafb888dd33292e2f0ea227721be7b1b73fe3..e98b95eff8c98344140c462e06b136747ea459bc 100644 (file)
--- a/fs/smb/client/cifstransport.c
+++ b/fs/smb/client/cifstransport.c
@@ -46,6 +46,7 @@ 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);
@@ -345,16 +346,15 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses,
        rc = wait_for_response(server, midQ);
        if (rc != 0) {
                send_cancel(server, &rqst, midQ);
-               spin_lock(&server->mid_queue_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_queue_lock);
+                       spin_unlock(&midQ->mid_lock);
                        add_credits(server, &credits, 0);
                        return rc;
                }
-               spin_unlock(&server->mid_queue_lock);
+               spin_unlock(&midQ->mid_lock);
        }
 
        rc = cifs_sync_mid_result(midQ, server);
@@ -527,15 +527,14 @@ 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_queue_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_queue_lock);
+                               spin_unlock(&midQ->mid_lock);
                                return rc;
                        }
-                       spin_unlock(&server->mid_queue_lock);
+                       spin_unlock(&midQ->mid_lock);
                }
 
                /* We got the response - restart system call. */

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 587845a2452d7aadea18d96343805f199b66803e..281ccbeea7191a130b299a4264fc87eef90514f6 100644 (file)
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -335,7 +335,7 @@ cifs_abort_connection(struct TCP_Server_Info *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);
        }
 
@@ -919,7 +919,7 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
                                list_del_init(&mid->qhead);
                                mid->mid_rc = mid_rc;
                                mid->mid_state = MID_RC;
-                               mid->callback(mid);
+                               mid_execute_callback(mid);
                                release_mid(mid);
                        }
 
@@ -1117,7 +1117,7 @@ clean_demultiplex_info(struct TCP_Server_Info *server)
                        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 */
@@ -1394,7 +1394,7 @@ next_pdu:
                                }
 
                                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 &&

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 218b6ce7ff3a52636774905725bf11812cf2ee59..3b251de874ec2ba4e1814de737affa2d094ec32f 100644 (file)
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4814,7 +4814,7 @@ static void smb2_decrypt_offload(struct work_struct *work)
                                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) {
@@ -4822,7 +4822,7 @@ static void smb2_decrypt_offload(struct work_struct *work)
                                mid->mid_state = MID_RETRY_NEEDED;
                                spin_unlock(&dw->server->mid_queue_lock);
                                spin_unlock(&dw->server->srv_lock);
-                               mid->callback(mid);
+                               mid_execute_callback(mid);
                        } else {
                                spin_lock(&dw->server->mid_queue_lock);
                                mid->mid_state = MID_REQUEST_SUBMITTED;

diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c
index ff9ef7fcd0105e4bd881bf2b2ebdaeb60d7f4d83..bc0e92eb2b64ae6e4fd2af87ed1a7f056876b19d 100644 (file)
--- a/fs/smb/client/smb2transport.c
+++ b/fs/smb/client/smb2transport.c
@@ -771,6 +771,7 @@ 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;

diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 32d528b4dd8354d2d84644361b5838044fca5c9f..a61ba7f3fb86bd0925180918993a22016069bc7d 100644 (file)
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -1005,15 +1005,14 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
                        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_queue_lock);
+                       spin_lock(&midQ[i]->mid_lock);
                        midQ[i]->wait_cancelled = true;
-                       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_queue_lock);
+                       spin_unlock(&midQ[i]->mid_lock);
                }
        }