| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250804134006.3609555-3-wangzhaolong@huaweicloud.com>
Date: Mon, 4 Aug 2025 21:40:04 +0800
From: Wang Zhaolong <wangzhaolong@...weicloud.com>
To: sfrench@...ba.org,
pshilov@...rosoft.com
Cc: linux-cifs@...r.kernel.org,
samba-technical@...ts.samba.org,
linux-kernel@...r.kernel.org,
chengzhihao1@...wei.com,
wangzhaolong@...weicloud.com,
yi.zhang@...wei.com,
yangerkun@...wei.com
Subject: [PATCH 2/4] smb: client: add mid_counter_lock to protect the mid counter counter
This is step 2/4 of a patch series to fix mid_q_entry memory leaks
caused by race conditions in callback execution.
Add a dedicated mid_counter_lock to protect current_mid counter,
separating it from mid_queue_lock which protects pending_mid_q
operations. This reduces lock contention and prepares for finer-
grained locking in subsequent patches.
Changes:
- Add TCP_Server_Info->mid_counter_lock spinlock
- Rename CurrentMid to current_mid for consistency
- Use mid_counter_lock to protect current_mid access
- Update locking documentation in cifsglob.h
This separation allows mid allocation to proceed without blocking
queue operations, improving performance under heavy load.
Signed-off-by: Wang Zhaolong <wangzhaolong@...weicloud.com>
---
fs/smb/client/cifsglob.h | 5 +++--
fs/smb/client/connect.c | 5 +++--
fs/smb/client/smb1ops.c | 11 ++++++-----
fs/smb/client/smb2ops.c | 40 +++++++++++++++++++--------------------
fs/smb/client/transport.c | 12 ++++++------
5 files changed, 38 insertions(+), 35 deletions(-)
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index ecd568793ce7..1844afdf1e41 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -731,10 +731,11 @@ struct TCP_Server_Info {
struct net *net;
#endif
wait_queue_head_t response_q;
wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
spinlock_t mid_queue_lock; /* protect mid queue */
+ spinlock_t mid_counter_lock;
struct list_head pending_mid_q;
bool noblocksnd; /* use blocking sendmsg */
bool noautotune; /* do not autotune send buf sizes */
bool nosharesock;
bool tcp_nodelay;
@@ -768,11 +769,11 @@ struct TCP_Server_Info {
unsigned int max_rw; /* maxRw specifies the maximum */
/* message size the server can send or receive for */
/* SMB_COM_WRITE_RAW or SMB_COM_READ_RAW. */
unsigned int capabilities; /* selective disabling of caps by smb sess */
int timeAdj; /* Adjust for difference in server time zone in sec */
- __u64 CurrentMid; /* multiplex id - rotating counter, protected by GlobalMid_Lock */
+ __u64 current_mid; /* multiplex id - rotating counter, protected by mid_counter_lock */
char cryptkey[CIFS_CRYPTO_KEY_SIZE]; /* used by ntlm, ntlmv2 etc */
/* 16th byte of RFC1001 workstation name is always null */
char workstation_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
__u32 sequence_number; /* for signing, protected by srv_mutex */
__u32 reconnect_instance; /* incremented on each reconnect */
@@ -2006,12 +2007,12 @@ require use of the stronger protocol */
* GlobalMid_Lock GlobalMaxActiveXid init_cifs
* GlobalCurrentXid
* GlobalTotalActiveXid
* 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
- * ->CurrentMid
* (any changes in mid_q_entry fields)
+ * 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
* ->credits
* ->echo_credits
* ->oplock_credits
* ->reconnect_instance
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index e4b577ca48d5..74ad5881ee45 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -356,11 +356,11 @@ static bool cifs_tcp_ses_needs_reconnect(struct TCP_Server_Info *server, int num
wake_up(&server->response_q);
return false;
}
cifs_dbg(FYI, "Mark tcp session as need reconnect\n");
- trace_smb3_reconnect(server->CurrentMid, server->conn_id,
+ trace_smb3_reconnect(server->current_mid, server->conn_id,
server->hostname);
server->tcpStatus = CifsNeedReconnect;
spin_unlock(&server->srv_lock);
return true;
@@ -1240,11 +1240,11 @@ smb2_add_credits_from_hdr(char *buffer, struct TCP_Server_Info *server)
scredits = server->credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
wake_up(&server->request_q);
- trace_smb3_hdr_credits(server->CurrentMid,
+ trace_smb3_hdr_credits(server->current_mid,
server->conn_id, server->hostname, scredits,
le16_to_cpu(shdr->CreditRequest), in_flight);
cifs_server_dbg(FYI, "%s: added %u credits total=%d\n",
__func__, le16_to_cpu(shdr->CreditRequest),
scredits);
@@ -1821,10 +1821,11 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
tcp_ses->lstrp = jiffies;
tcp_ses->compression.requested = ctx->compress;
spin_lock_init(&tcp_ses->req_lock);
spin_lock_init(&tcp_ses->srv_lock);
spin_lock_init(&tcp_ses->mid_queue_lock);
+ spin_lock_init(&tcp_ses->mid_counter_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);
INIT_DELAYED_WORK(&tcp_ses->reconnect, smb2_reconnect_server);
mutex_init(&tcp_ses->reconnect_mutex);
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index a1442f697706..13f600a3d0c4 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -167,14 +167,13 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
{
__u64 mid = 0;
__u16 last_mid, cur_mid;
bool collision, reconnect = false;
- spin_lock(&server->mid_queue_lock);
-
+ spin_lock(&server->mid_counter_lock);
/* mid is 16 bit only for CIFS/SMB */
- cur_mid = (__u16)((server->CurrentMid) & 0xffff);
+ cur_mid = (__u16)((server->current_mid) & 0xffff);
/* we do not want to loop forever */
last_mid = cur_mid;
cur_mid++;
/* avoid 0xFFFF MID */
if (cur_mid == 0xffff)
@@ -196,19 +195,21 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
collision = false;
if (cur_mid == 0)
cur_mid++;
num_mids = 0;
+ spin_lock(&server->mid_queue_lock);
list_for_each_entry(mid_entry, &server->pending_mid_q, qhead) {
++num_mids;
if (mid_entry->mid == cur_mid &&
mid_entry->mid_state == MID_REQUEST_SUBMITTED) {
/* This mid is in use, try a different one */
collision = true;
break;
}
}
+ spin_unlock(&server->mid_queue_lock);
/*
* if we have more than 32k mids in the list, then something
* is very wrong. Possibly a local user is trying to DoS the
* box by issuing long-running calls and SIGKILL'ing them. If
@@ -221,16 +222,16 @@ cifs_get_next_mid(struct TCP_Server_Info *server)
if (num_mids > 32768)
reconnect = true;
if (!collision) {
mid = (__u64)cur_mid;
- server->CurrentMid = mid;
+ server->current_mid = mid;
break;
}
cur_mid++;
}
- spin_unlock(&server->mid_queue_lock);
+ spin_unlock(&server->mid_counter_lock);
if (reconnect) {
cifs_signal_cifsd_for_reconnect(server, false);
}
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index c714707249c7..da2cb9585404 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -89,11 +89,11 @@ smb2_add_credits(struct TCP_Server_Info *server,
reconnect_detected = true;
if (*val > 65000) {
*val = 65000; /* Don't get near 64K credits, avoid srv bugs */
pr_warn_once("server overflowed SMB3 credits\n");
- trace_smb3_overflow_credits(server->CurrentMid,
+ trace_smb3_overflow_credits(server->current_mid,
server->conn_id, server->hostname, *val,
add, server->in_flight);
}
if (credits->in_flight_check > 1) {
pr_warn_once("rreq R=%08x[%x] Credits not in flight\n",
@@ -134,19 +134,19 @@ smb2_add_credits(struct TCP_Server_Info *server,
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
wake_up(&server->request_q);
if (reconnect_detected) {
- trace_smb3_reconnect_detected(server->CurrentMid,
+ trace_smb3_reconnect_detected(server->current_mid,
server->conn_id, server->hostname, scredits, add, in_flight);
cifs_dbg(FYI, "trying to put %d credits from the old server instance %d\n",
add, instance);
}
if (reconnect_with_invalid_credits) {
- trace_smb3_reconnect_with_invalid_credits(server->CurrentMid,
+ trace_smb3_reconnect_with_invalid_credits(server->current_mid,
server->conn_id, server->hostname, scredits, add, in_flight);
cifs_dbg(FYI, "Negotiate operation when server credits is non-zero. Optype: %d, server credits: %d, credits added: %d\n",
optype, scredits, add);
}
@@ -174,11 +174,11 @@ smb2_add_credits(struct TCP_Server_Info *server,
default:
/* change_conf rebalanced credits for different types */
break;
}
- trace_smb3_add_credits(server->CurrentMid,
+ trace_smb3_add_credits(server->current_mid,
server->conn_id, server->hostname, scredits, add, in_flight);
cifs_dbg(FYI, "%s: added %u credits total=%d\n", __func__, add, scredits);
}
static void
@@ -201,11 +201,11 @@ smb2_set_credits(struct TCP_Server_Info *server, const int val)
}
scredits = server->credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
- trace_smb3_set_credits(server->CurrentMid,
+ trace_smb3_set_credits(server->current_mid,
server->conn_id, server->hostname, scredits, val, in_flight);
cifs_dbg(FYI, "%s: set %u credits\n", __func__, val);
/* don't log while holding the lock */
if (val == 1)
@@ -286,11 +286,11 @@ smb2_wait_mtu_credits(struct TCP_Server_Info *server, size_t size,
}
scredits = server->credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
- trace_smb3_wait_credits(server->CurrentMid,
+ trace_smb3_wait_credits(server->current_mid,
server->conn_id, server->hostname, scredits, -(credits->value), in_flight);
cifs_dbg(FYI, "%s: removed %u credits total=%d\n",
__func__, credits->value, scredits);
return rc;
@@ -314,11 +314,11 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
subreq->subreq.debug_index,
credits->value,
server->credits, server->in_flight,
new_val - credits->value,
cifs_trace_rw_credits_no_adjust_up);
- trace_smb3_too_many_credits(server->CurrentMid,
+ trace_smb3_too_many_credits(server->current_mid,
server->conn_id, server->hostname, 0, credits->value - new_val, 0);
cifs_server_dbg(VFS, "R=%x[%x] request has less credits (%d) than required (%d)",
subreq->rreq->debug_id, subreq->subreq.debug_index,
credits->value, new_val);
@@ -336,11 +336,11 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
subreq->subreq.debug_index,
credits->value,
server->credits, server->in_flight,
new_val - credits->value,
cifs_trace_rw_credits_old_session);
- trace_smb3_reconnect_detected(server->CurrentMid,
+ trace_smb3_reconnect_detected(server->current_mid,
server->conn_id, server->hostname, scredits,
credits->value - new_val, in_flight);
cifs_server_dbg(VFS, "R=%x[%x] trying to return %d credits to old session\n",
subreq->rreq->debug_id, subreq->subreq.debug_index,
credits->value - new_val);
@@ -356,11 +356,11 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
scredits = server->credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
wake_up(&server->request_q);
- trace_smb3_adj_credits(server->CurrentMid,
+ trace_smb3_adj_credits(server->current_mid,
server->conn_id, server->hostname, scredits,
credits->value - new_val, in_flight);
cifs_dbg(FYI, "%s: adjust added %u credits total=%d\n",
__func__, credits->value - new_val, scredits);
@@ -372,23 +372,23 @@ smb2_adjust_credits(struct TCP_Server_Info *server,
static __u64
smb2_get_next_mid(struct TCP_Server_Info *server)
{
__u64 mid;
/* for SMB2 we need the current value */
- spin_lock(&server->mid_queue_lock);
- mid = server->CurrentMid++;
- spin_unlock(&server->mid_queue_lock);
+ spin_lock(&server->mid_counter_lock);
+ mid = server->current_mid++;
+ spin_unlock(&server->mid_counter_lock);
return mid;
}
static void
smb2_revert_current_mid(struct TCP_Server_Info *server, const unsigned int val)
{
- spin_lock(&server->mid_queue_lock);
- if (server->CurrentMid >= val)
- server->CurrentMid -= val;
- spin_unlock(&server->mid_queue_lock);
+ spin_lock(&server->mid_counter_lock);
+ if (server->current_mid >= val)
+ server->current_mid -= val;
+ spin_unlock(&server->mid_counter_lock);
}
static struct mid_q_entry *
__smb2_find_mid(struct TCP_Server_Info *server, char *buf, bool dequeue)
{
@@ -458,13 +458,13 @@ smb2_negotiate(const unsigned int xid,
struct cifs_ses *ses,
struct TCP_Server_Info *server)
{
int rc;
- spin_lock(&server->mid_queue_lock);
- server->CurrentMid = 0;
- spin_unlock(&server->mid_queue_lock);
+ spin_lock(&server->mid_counter_lock);
+ server->current_mid = 0;
+ spin_unlock(&server->mid_counter_lock);
rc = SMB2_negotiate(xid, ses, server);
return rc;
}
static inline unsigned int
@@ -2496,11 +2496,11 @@ smb2_is_status_pending(char *buf, struct TCP_Server_Info *server)
scredits = server->credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
wake_up(&server->request_q);
- trace_smb3_pend_credits(server->CurrentMid,
+ trace_smb3_pend_credits(server->current_mid,
server->conn_id, server->hostname, scredits,
le16_to_cpu(shdr->CreditRequest), in_flight);
cifs_dbg(FYI, "%s: status pending add %u credits total=%d\n",
__func__, le16_to_cpu(shdr->CreditRequest), scredits);
}
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 12dc927aa4a2..8037accc3987 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -395,11 +395,11 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
* If we have only sent part of an SMB then the next SMB could
* be taken as the remainder of this one. We need to kill the
* socket so the server throws away the partial SMB
*/
cifs_signal_cifsd_for_reconnect(server, false);
- trace_smb3_partial_send_reconnect(server->CurrentMid,
+ trace_smb3_partial_send_reconnect(server->current_mid,
server->conn_id, server->hostname);
}
smbd_done:
/*
* there's hardly any use for the layers above to know the
@@ -507,11 +507,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
*instance = server->reconnect_instance;
scredits = *credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
- trace_smb3_nblk_credits(server->CurrentMid,
+ trace_smb3_nblk_credits(server->current_mid,
server->conn_id, server->hostname, scredits, -1, in_flight);
cifs_dbg(FYI, "%s: remove %u credits total=%d\n",
__func__, 1, scredits);
return 0;
@@ -540,11 +540,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
spin_lock(&server->req_lock);
scredits = *credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
- trace_smb3_credit_timeout(server->CurrentMid,
+ trace_smb3_credit_timeout(server->current_mid,
server->conn_id, server->hostname, scredits,
num_credits, in_flight);
cifs_server_dbg(VFS, "wait timed out after %d ms\n",
timeout);
return -EBUSY;
@@ -583,11 +583,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
scredits = *credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
trace_smb3_credit_timeout(
- server->CurrentMid,
+ server->current_mid,
server->conn_id, server->hostname,
scredits, num_credits, in_flight);
cifs_server_dbg(VFS, "wait timed out after %d ms\n",
timeout);
return -EBUSY;
@@ -613,11 +613,11 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits,
}
scredits = *credits;
in_flight = server->in_flight;
spin_unlock(&server->req_lock);
- trace_smb3_waitff_credits(server->CurrentMid,
+ trace_smb3_waitff_credits(server->current_mid,
server->conn_id, server->hostname, scredits,
-(num_credits), in_flight);
cifs_dbg(FYI, "%s: remove %u credits total=%d\n",
__func__, num_credits, scredits);
break;
@@ -664,11 +664,11 @@ wait_for_compound_request(struct TCP_Server_Info *server, int num,
* Return immediately if no requests in flight since we will be
* stuck on waiting for credits.
*/
if (server->in_flight == 0) {
spin_unlock(&server->req_lock);
- trace_smb3_insufficient_credits(server->CurrentMid,
+ trace_smb3_insufficient_credits(server->current_mid,
server->conn_id, server->hostname, scredits,
num, in_flight);
cifs_dbg(FYI, "%s: %d requests in flight, needed %d total=%d\n",
__func__, in_flight, num, scredits);
return -EDEADLK;
--
2.39.2
Powered by blists - more mailing lists