Skip to content

Commit 5c88b37

Browse files
committed
smb: client: smb: client: eliminate mid_flags field
JIRA: https://issues.redhat.com/browse/RHEL-109507 commit 3fd8ec2 Author: Wang Zhaolong <[email protected]> Date: Mon Aug 4 21:40:05 2025 +0800 smb: client: smb: client: eliminate mid_flags field This is step 3/4 of a patch series to fix mid_q_entry memory leaks caused by race conditions in callback execution. Replace the mid_flags bitmask with dedicated boolean fields to simplify locking logic and improve code readability: - Replace MID_DELETED with bool deleted_from_q - Replace MID_WAIT_CANCELLED with bool wait_cancelled - Remove mid_flags field entirely The new boolean fields have clearer semantics: - deleted_from_q: whether mid has been removed from pending_mid_q - wait_cancelled: whether request was cancelled during wait This change reduces memory usage (from 4-byte bitmask to 2 boolean flags) and eliminates confusion about which lock protects which flag bits, preparing for per-mid locking in the next patch. Signed-off-by: Wang Zhaolong <[email protected]> Acked-by: Enzo Matsumiya <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Paulo Alcantara <[email protected]>
1 parent 9e6368d commit 5c88b37

File tree

4 files changed

+16
-19
lines changed

4 files changed

+16
-19
lines changed

fs/smb/client/cifsglob.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,9 +1787,10 @@ struct mid_q_entry {
17871787
unsigned int resp_buf_size;
17881788
int mid_state; /* wish this were enum but can not pass to wait_event */
17891789
int mid_rc; /* rc for MID_RC */
1790-
unsigned int mid_flags;
17911790
__le16 command; /* smb command code */
17921791
unsigned int optype; /* operation type */
1792+
bool wait_cancelled:1; /* Cancelled while waiting for response */
1793+
bool deleted_from_q:1; /* Whether Mid has been dequeued frem pending_mid_q */
17931794
bool large_buf:1; /* if valid response, is pointer to large buf */
17941795
bool multiRsp:1; /* multiple trans2 responses for one request */
17951796
bool multiEnd:1; /* both received */
@@ -1951,10 +1952,6 @@ static inline bool is_replayable_error(int error)
19511952
#define MID_RESPONSE_READY 0x40 /* ready for other process handle the rsp */
19521953
#define MID_RC 0x80 /* mid_rc contains custom rc */
19531954

1954-
/* Flags */
1955-
#define MID_WAIT_CANCELLED 1 /* Cancelled while waiting for response */
1956-
#define MID_DELETED 2 /* Mid has been dequeued/deleted */
1957-
19581955
/* Types of response buffer returned from SendReceive2 */
19591956
#define CIFS_NO_BUFFER 0 /* Response buffer not returned */
19601957
#define CIFS_SMALL_BUFFER 1
@@ -2066,7 +2063,7 @@ require use of the stronger protocol */
20662063
* GlobalTotalActiveXid
20672064
* TCP_Server_Info->srv_lock (anything in struct not protected by another lock and can change)
20682065
* TCP_Server_Info->mid_queue_lock TCP_Server_Info->pending_mid_q cifs_get_tcp_session
2069-
* (any changes in mid_q_entry fields)
2066+
* mid_q_entry->deleted_from_q
20702067
* TCP_Server_Info->mid_counter_lock TCP_Server_Info->current_mid cifs_get_tcp_session
20712068
* TCP_Server_Info->req_lock TCP_Server_Info->in_flight cifs_get_tcp_session
20722069
* ->credits

fs/smb/client/connect.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
327327
if (mid->mid_state == MID_REQUEST_SUBMITTED)
328328
mid->mid_state = MID_RETRY_NEEDED;
329329
list_move(&mid->qhead, &retry_list);
330-
mid->mid_flags |= MID_DELETED;
330+
mid->deleted_from_q = true;
331331
}
332332
spin_unlock(&server->mid_queue_lock);
333333
cifs_server_unlock(server);
@@ -890,7 +890,7 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
890890
list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
891891
kref_get(&mid->refcount);
892892
list_move(&mid->qhead, &dispose_list);
893-
mid->mid_flags |= MID_DELETED;
893+
mid->deleted_from_q = true;
894894
}
895895
spin_unlock(&server->mid_queue_lock);
896896

@@ -968,12 +968,12 @@ dequeue_mid(struct mid_q_entry *mid, bool malformed)
968968
* Trying to handle/dequeue a mid after the send_recv()
969969
* function has finished processing it is a bug.
970970
*/
971-
if (mid->mid_flags & MID_DELETED) {
971+
if (mid->deleted_from_q == true) {
972972
spin_unlock(&mid->server->mid_queue_lock);
973973
pr_warn_once("trying to dequeue a deleted mid\n");
974974
} else {
975975
list_del_init(&mid->qhead);
976-
mid->mid_flags |= MID_DELETED;
976+
mid->deleted_from_q = true;
977977
spin_unlock(&mid->server->mid_queue_lock);
978978
}
979979
}
@@ -1110,7 +1110,7 @@ clean_demultiplex_info(struct TCP_Server_Info *server)
11101110
kref_get(&mid_entry->refcount);
11111111
mid_entry->mid_state = MID_SHUTDOWN;
11121112
list_move(&mid_entry->qhead, &dispose_list);
1113-
mid_entry->mid_flags |= MID_DELETED;
1113+
mid_entry->deleted_from_q = true;
11141114
}
11151115
spin_unlock(&server->mid_queue_lock);
11161116

fs/smb/client/smb2ops.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ __smb2_find_mid(struct TCP_Server_Info *server, char *buf, bool dequeue)
373373
kref_get(&mid->refcount);
374374
if (dequeue) {
375375
list_del_init(&mid->qhead);
376-
mid->mid_flags |= MID_DELETED;
376+
mid->deleted_from_q = true;
377377
}
378378
spin_unlock(&server->mid_queue_lock);
379379
return mid;
@@ -4753,7 +4753,7 @@ static void smb2_decrypt_offload(struct work_struct *work)
47534753
} else {
47544754
spin_lock(&dw->server->mid_queue_lock);
47554755
mid->mid_state = MID_REQUEST_SUBMITTED;
4756-
mid->mid_flags &= ~(MID_DELETED);
4756+
mid->deleted_from_q = false;
47574757
list_add_tail(&mid->qhead,
47584758
&dw->server->pending_mid_q);
47594759
spin_unlock(&dw->server->mid_queue_lock);

fs/smb/client/transport.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ void __release_mid(struct kref *refcount)
8989
#endif
9090
struct TCP_Server_Info *server = midEntry->server;
9191

92-
if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) &&
92+
if (midEntry->resp_buf && (midEntry->wait_cancelled) &&
9393
(midEntry->mid_state == MID_RESPONSE_RECEIVED ||
9494
midEntry->mid_state == MID_RESPONSE_READY) &&
9595
server->ops->handle_cancelled_mid)
@@ -161,9 +161,9 @@ void
161161
delete_mid(struct mid_q_entry *mid)
162162
{
163163
spin_lock(&mid->server->mid_queue_lock);
164-
if (!(mid->mid_flags & MID_DELETED)) {
164+
if (mid->deleted_from_q == false) {
165165
list_del_init(&mid->qhead);
166-
mid->mid_flags |= MID_DELETED;
166+
mid->deleted_from_q = true;
167167
}
168168
spin_unlock(&mid->server->mid_queue_lock);
169169

@@ -925,9 +925,9 @@ cifs_sync_mid_result(struct mid_q_entry *mid, struct TCP_Server_Info *server)
925925
rc = mid->mid_rc;
926926
break;
927927
default:
928-
if (!(mid->mid_flags & MID_DELETED)) {
928+
if (mid->deleted_from_q == false) {
929929
list_del_init(&mid->qhead);
930-
mid->mid_flags |= MID_DELETED;
930+
mid->deleted_from_q = true;
931931
}
932932
spin_unlock(&server->mid_queue_lock);
933933
cifs_server_dbg(VFS, "%s: invalid mid state mid=%llu state=%d\n",
@@ -1241,7 +1241,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
12411241
midQ[i]->mid, le16_to_cpu(midQ[i]->command));
12421242
send_cancel(server, &rqst[i], midQ[i]);
12431243
spin_lock(&server->mid_queue_lock);
1244-
midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
1244+
midQ[i]->wait_cancelled = true;
12451245
if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED ||
12461246
midQ[i]->mid_state == MID_RESPONSE_RECEIVED) {
12471247
midQ[i]->callback = cifs_cancelled_callback;

0 commit comments

Comments
 (0)