Gh0st
/
linux
mirror of https://github.com/torvalds/linux.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 <wangzhaolong@huaweicloud.com>
Acked-by: Enzo Matsumiya <ematsumiya@suse.de>
Signed-off-by: Steve French <stfrench@microsoft.com>
This commit is contained in:
Wang Zhaolong 2025-08-04 21:40:05 +08:00 committed by Steve French
parent 9bd42798d5
commit 3fd8ec2fc9
4 changed files with 16 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
fs/smb/client/cifsglob.h
View File

@ -1730,9 +1730,10 @@ struct mid_q_entry {
unsigned int resp_buf_size; unsigned int resp_buf_size;
int mid_state; /* wish this were enum but can not pass to wait_event */ int mid_state; /* wish this were enum but can not pass to wait_event */
int mid_rc; /* rc for MID_RC */ int mid_rc; /* rc for MID_RC */
unsigned int mid_flags;
__le16 command; /* smb command code */ __le16 command; /* smb command code */
unsigned int optype; /* operation type */ unsigned int optype; /* operation type */
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 */ bool large_buf:1; /* if valid response, is pointer to large buf */
bool multiRsp:1; /* multiple trans2 responses for one request */ bool multiRsp:1; /* multiple trans2 responses for one request */
bool multiEnd:1; /* both received */ bool multiEnd:1; /* both received */
@ -1894,10 +1895,6 @@ static inline bool is_replayable_error(int error)
#define MID_RESPONSE_READY 0x40 /* ready for other process handle the rsp */ #define MID_RESPONSE_READY 0x40 /* ready for other process handle the rsp */
#define MID_RC 0x80 /* mid_rc contains custom rc */ #define MID_RC 0x80 /* mid_rc contains custom rc */
/* Flags */
#define MID_WAIT_CANCELLED 1 /* Cancelled while waiting for response */
#define MID_DELETED 2 /* Mid has been dequeued/deleted */
/* Types of response buffer returned from SendReceive2 */ /* Types of response buffer returned from SendReceive2 */
#define CIFS_NO_BUFFER 0 /* Response buffer not returned */ #define CIFS_NO_BUFFER 0 /* Response buffer not returned */
#define CIFS_SMALL_BUFFER 1 #define CIFS_SMALL_BUFFER 1
@ -2009,7 +2006,7 @@ require use of the stronger protocol */
* GlobalTotalActiveXid * GlobalTotalActiveXid
* TCP_Server_Info->srv_lock (anything in struct not protected by another lock and can change) * TCP_Server_Info->srv_lock (anything in struct not protected by another lock and can change)
* TCP_Server_Info->mid_queue_lock TCP_Server_Info->pending_mid_q cifs_get_tcp_session * TCP_Server_Info->mid_queue_lock TCP_Server_Info->pending_mid_q cifs_get_tcp_session
* (any changes in mid_q_entry fields) * mid_q_entry->deleted_from_q
* TCP_Server_Info->mid_counter_lock TCP_Server_Info->current_mid cifs_get_tcp_session * TCP_Server_Info->mid_counter_lock TCP_Server_Info->current_mid cifs_get_tcp_session
* TCP_Server_Info->req_lock TCP_Server_Info->in_flight cifs_get_tcp_session * TCP_Server_Info->req_lock TCP_Server_Info->in_flight cifs_get_tcp_session
* ->credits * ->credits

10
fs/smb/client/connect.c
View File

@ -327,7 +327,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
if (mid->mid_state == MID_REQUEST_SUBMITTED) if (mid->mid_state == MID_REQUEST_SUBMITTED)
mid->mid_state = MID_RETRY_NEEDED; mid->mid_state = MID_RETRY_NEEDED;
list_move(&mid->qhead, &retry_list); list_move(&mid->qhead, &retry_list);
mid->mid_flags |= MID_DELETED; mid->deleted_from_q = true;
} }
spin_unlock(&server->mid_queue_lock); spin_unlock(&server->mid_queue_lock);
cifs_server_unlock(server); cifs_server_unlock(server);
@ -888,7 +888,7 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) { list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
kref_get(&mid->refcount); kref_get(&mid->refcount);
list_move(&mid->qhead, &dispose_list); list_move(&mid->qhead, &dispose_list);
mid->mid_flags |= MID_DELETED; mid->deleted_from_q = true;
} }
spin_unlock(&server->mid_queue_lock); spin_unlock(&server->mid_queue_lock);
@ -966,12 +966,12 @@ dequeue_mid(struct mid_q_entry *mid, bool malformed)
* Trying to handle/dequeue a mid after the send_recv() * Trying to handle/dequeue a mid after the send_recv()
* function has finished processing it is a bug. * function has finished processing it is a bug.
*/ */
if (mid->mid_flags & MID_DELETED) { if (mid->deleted_from_q == true) {
spin_unlock(&mid->server->mid_queue_lock); spin_unlock(&mid->server->mid_queue_lock);
pr_warn_once("trying to dequeue a deleted mid\n"); pr_warn_once("trying to dequeue a deleted mid\n");
} else { } else {
list_del_init(&mid->qhead); list_del_init(&mid->qhead);
mid->mid_flags |= MID_DELETED; mid->deleted_from_q = true;
spin_unlock(&mid->server->mid_queue_lock); spin_unlock(&mid->server->mid_queue_lock);
} }
} }
@ -1108,7 +1108,7 @@ clean_demultiplex_info(struct TCP_Server_Info *server)
kref_get(&mid_entry->refcount); kref_get(&mid_entry->refcount);
mid_entry->mid_state = MID_SHUTDOWN; mid_entry->mid_state = MID_SHUTDOWN;
list_move(&mid_entry->qhead, &dispose_list); list_move(&mid_entry->qhead, &dispose_list);
mid_entry->mid_flags |= MID_DELETED; mid_entry->deleted_from_q = true;
} }
spin_unlock(&server->mid_queue_lock); spin_unlock(&server->mid_queue_lock);

4
fs/smb/client/smb2ops.c
View File

@ -409,7 +409,7 @@ __smb2_find_mid(struct TCP_Server_Info *server, char *buf, bool dequeue)
kref_get(&mid->refcount); kref_get(&mid->refcount);
if (dequeue) { if (dequeue) {
list_del_init(&mid->qhead); list_del_init(&mid->qhead);
mid->mid_flags |= MID_DELETED; mid->deleted_from_q = true;
} }
spin_unlock(&server->mid_queue_lock); spin_unlock(&server->mid_queue_lock);
return mid; return mid;
@ -4817,7 +4817,7 @@ static void smb2_decrypt_offload(struct work_struct *work)
} else { } else {
spin_lock(&dw->server->mid_queue_lock); spin_lock(&dw->server->mid_queue_lock);
mid->mid_state = MID_REQUEST_SUBMITTED; mid->mid_state = MID_REQUEST_SUBMITTED;
mid->mid_flags &= ~(MID_DELETED); mid->deleted_from_q = false;
list_add_tail(&mid->qhead, list_add_tail(&mid->qhead,
&dw->server->pending_mid_q); &dw->server->pending_mid_q);
spin_unlock(&dw->server->mid_queue_lock); spin_unlock(&dw->server->mid_queue_lock);

12
fs/smb/client/transport.c
View File

@ -89,7 +89,7 @@ void __release_mid(struct kref *refcount)
#endif #endif
struct TCP_Server_Info *server = midEntry->server; struct TCP_Server_Info *server = midEntry->server;
if (midEntry->resp_buf && (midEntry->mid_flags & MID_WAIT_CANCELLED) && if (midEntry->resp_buf && (midEntry->wait_cancelled) &&
(midEntry->mid_state == MID_RESPONSE_RECEIVED || (midEntry->mid_state == MID_RESPONSE_RECEIVED ||
midEntry->mid_state == MID_RESPONSE_READY) && midEntry->mid_state == MID_RESPONSE_READY) &&
server->ops->handle_cancelled_mid) server->ops->handle_cancelled_mid)
@ -161,9 +161,9 @@ void
delete_mid(struct mid_q_entry *mid) delete_mid(struct mid_q_entry *mid)
{ {
spin_lock(&mid->server->mid_queue_lock); spin_lock(&mid->server->mid_queue_lock);
if (!(mid->mid_flags & MID_DELETED)) { if (mid->deleted_from_q == false) {
list_del_init(&mid->qhead); list_del_init(&mid->qhead);
mid->mid_flags |= MID_DELETED; mid->deleted_from_q = true;
} }
spin_unlock(&mid->server->mid_queue_lock); spin_unlock(&mid->server->mid_queue_lock);
@ -898,9 +898,9 @@ cifs_sync_mid_result(struct mid_q_entry *mid, struct TCP_Server_Info *server)
rc = mid->mid_rc; rc = mid->mid_rc;
break; break;
default: default:
if (!(mid->mid_flags & MID_DELETED)) { if (mid->deleted_from_q == false) {
list_del_init(&mid->qhead); list_del_init(&mid->qhead);
mid->mid_flags |= MID_DELETED; mid->deleted_from_q = true;
} }
spin_unlock(&server->mid_queue_lock); spin_unlock(&server->mid_queue_lock);
cifs_server_dbg(VFS, "%s: invalid mid state mid=%llu state=%d\n", cifs_server_dbg(VFS, "%s: invalid mid state mid=%llu state=%d\n",
@ -1214,7 +1214,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
midQ[i]->mid, le16_to_cpu(midQ[i]->command)); midQ[i]->mid, le16_to_cpu(midQ[i]->command));
send_cancel(server, &rqst[i], midQ[i]); send_cancel(server, &rqst[i], midQ[i]);
spin_lock(&server->mid_queue_lock); spin_lock(&server->mid_queue_lock);
midQ[i]->mid_flags |= MID_WAIT_CANCELLED; midQ[i]->wait_cancelled = true;
if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED || if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED ||
midQ[i]->mid_state == MID_RESPONSE_RECEIVED) { midQ[i]->mid_state == MID_RESPONSE_RECEIVED) {
midQ[i]->callback = cifs_cancelled_callback; midQ[i]->callback = cifs_cancelled_callback;