From: Shyam Prasad N sprasad@microsoft.com
mainline inclusion from mainline-v5.16-rc1 commit 724244cdb3828522109c88e56a0242537aefabe9 category: bugfix bugzilla: 189952 CVE: CVE-2024-35999
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Introducing a new spin lock to protect all the channel related fields in a cifs_ses struct. This lock should be taken whenever dealing with the channel fields, and should be held only for very short intervals which will not sleep.
Currently, all channel related fields in cifs_ses structure are protected by session_mutex. However, this mutex is held for long periods (sometimes while waiting for a reply from server). This makes the codepath quite tricky to change.
Signed-off-by: Shyam Prasad N sprasad@microsoft.com Reviewed-by: Paulo Alcantara (SUSE) pc@cjr.nz Signed-off-by: Steve French stfrench@microsoft.com Conflicts: fs/cifs/cifs_debug.c fs/cifs/connect.c fs/cifs/sess.c [qyf: Just context differences] Signed-off-by: Yifan Qiao qiaoyifan4@huawei.com --- fs/cifs/cifsglob.h | 5 +++++ fs/cifs/cifs_debug.c | 2 ++ fs/cifs/connect.c | 25 +++++++++++++++++++--- fs/cifs/misc.c | 1 + fs/cifs/sess.c | 50 +++++++++++++++++++++++++++++++++----------- fs/cifs/transport.c | 3 +++ 6 files changed, 71 insertions(+), 15 deletions(-)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index a6697954fd68..8825548b5b22 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -1021,16 +1021,21 @@ struct cifs_ses { * iface_lock should be taken when accessing any of these fields */ spinlock_t iface_lock; + /* ========= begin: protected by iface_lock ======== */ struct cifs_server_iface *iface_list; size_t iface_count; unsigned long iface_last_update; /* jiffies */ + /* ========= end: protected by iface_lock ======== */
+ spinlock_t chan_lock; + /* ========= begin: protected by chan_lock ======== */ #define CIFS_MAX_CHANNELS 16 struct cifs_chan chans[CIFS_MAX_CHANNELS]; struct cifs_chan *binding_chan; size_t chan_count; size_t chan_max; atomic_t chan_seq; /* round robin state */ + /* ========= end: protected by chan_lock ======== */ };
/* diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c index 53588d7517b4..14cdda453b13 100644 --- a/fs/cifs/cifs_debug.c +++ b/fs/cifs/cifs_debug.c @@ -403,12 +403,14 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) from_kuid(&init_user_ns, ses->linux_uid), from_kuid(&init_user_ns, ses->cred_uid));
+ spin_lock(&ses->chan_lock); if (ses->chan_count > 1) { seq_printf(m, "\n\n\tExtra Channels: %zu\n", ses->chan_count-1); for (j = 1; j < ses->chan_count; j++) cifs_dump_channel(m, j, &ses->chans[j]); } + spin_unlock(&ses->chan_lock);
seq_puts(m, "\n\n\tShares:"); j = 0; diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 090e595c921b..0519c20919aa 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -2723,8 +2723,12 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol) * If an existing session is limited to less channels than * requested, it should not be reused */ - if (ses->chan_max < vol->max_channels) + spin_lock(&ses->chan_lock); + if (ses->chan_max < vol->max_channels) { + spin_unlock(&ses->chan_lock); return 0; + } + spin_unlock(&ses->chan_lock);
switch (ses->sectype) { case Kerberos: @@ -2864,6 +2868,7 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol) void cifs_put_smb_ses(struct cifs_ses *ses) { unsigned int rc, xid; + unsigned int chan_count; struct TCP_Server_Info *server = ses->server;
cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count); @@ -2899,12 +2904,24 @@ void cifs_put_smb_ses(struct cifs_ses *ses) list_del_init(&ses->smb_ses_list); spin_unlock(&cifs_tcp_ses_lock);
+ spin_lock(&ses->chan_lock); + chan_count = ses->chan_count; + spin_unlock(&ses->chan_lock); + /* close any extra channels */ - if (ses->chan_count > 1) { + if (chan_count > 1) { int i;
- for (i = 1; i < ses->chan_count; i++) + for (i = 1; i < chan_count; i++) { + /* + * note: for now, we're okay accessing ses->chans + * without chan_lock. But when chans can go away, we'll + * need to introduce ref counting to make sure that chan + * is not freed from under us. + */ cifs_put_tcp_session(ses->chans[i].server, 0); + ses->chans[i].server = NULL; + } }
sesInfoFree(ses); @@ -3157,9 +3174,11 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info) mutex_lock(&ses->session_mutex);
/* add server as first channel */ + spin_lock(&ses->chan_lock); ses->chans[0].server = server; ses->chan_count = 1; ses->chan_max = volume_info->multichannel ? volume_info->max_channels:1; + spin_unlock(&ses->chan_lock);
rc = cifs_negotiate_protocol(xid, ses); if (!rc) diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 2d46018b0283..e74b03caa28b 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -87,6 +87,7 @@ sesInfoAlloc(void) INIT_LIST_HEAD(&ret_buf->tcon_list); mutex_init(&ret_buf->session_mutex); spin_lock_init(&ret_buf->iface_lock); + spin_lock_init(&ret_buf->chan_lock); } return ret_buf; } diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index cf6fd138d8d5..1eb275ebe1df 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -62,41 +62,53 @@ bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface) { int i;
+ spin_lock(&ses->chan_lock); for (i = 0; i < ses->chan_count; i++) { - if (is_server_using_iface(ses->chans[i].server, iface)) + if (is_server_using_iface(ses->chans[i].server, iface)) { + spin_unlock(&ses->chan_lock); return true; + } } + spin_unlock(&ses->chan_lock); return false; }
/* returns number of channels added */ int cifs_try_adding_channels(struct cifs_ses *ses) { - int old_chan_count = ses->chan_count; - int left = ses->chan_max - ses->chan_count; + int old_chan_count, new_chan_count; + int left; int i = 0; int rc = 0; int tries = 0; struct cifs_server_iface *ifaces = NULL; size_t iface_count;
+ if (ses->server->dialect < SMB30_PROT_ID) { + cifs_dbg(VFS, "multichannel is not supported on this protocol version, use 3.0 or above\n"); + return 0; + } + + spin_lock(&ses->chan_lock); + + new_chan_count = old_chan_count = ses->chan_count; + left = ses->chan_max - ses->chan_count; + if (left <= 0) { cifs_dbg(FYI, "ses already at max_channels (%zu), nothing to open\n", ses->chan_max); - return 0; - } - - if (ses->server->dialect < SMB30_PROT_ID) { - cifs_dbg(VFS, "multichannel is not supported on this protocol version, use 3.0 or above\n"); + spin_unlock(&ses->chan_lock); return 0; }
if (!(ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { cifs_dbg(VFS, "server %s does not support multichannel\n", ses->server->hostname); ses->chan_max = 1; + spin_unlock(&ses->chan_lock); return 0; } + spin_unlock(&ses->chan_lock);
/* * Make a copy of the iface list at the time and use that @@ -150,10 +162,11 @@ int cifs_try_adding_channels(struct cifs_ses *ses) cifs_dbg(FYI, "successfully opened new channel on iface#%d\n", i); left--; + new_chan_count++; }
kfree(ifaces); - return ses->chan_count - old_chan_count; + return new_chan_count - old_chan_count; }
/* @@ -165,16 +178,21 @@ cifs_ses_find_chan(struct cifs_ses *ses, struct TCP_Server_Info *server) { int i;
+ spin_lock(&ses->chan_lock); for (i = 0; i < ses->chan_count; i++) { - if (ses->chans[i].server == server) + if (ses->chans[i].server == server) { + spin_unlock(&ses->chan_lock); return &ses->chans[i]; + } } + spin_unlock(&ses->chan_lock); return NULL; }
int cifs_ses_add_channel(struct cifs_ses *ses, struct cifs_server_iface *iface) { + struct TCP_Server_Info *chan_server; struct cifs_chan *chan; struct smb_vol vol = {NULL}; static const char unc_fmt[] = "\%s\foo"; @@ -252,15 +270,20 @@ cifs_ses_add_channel(struct cifs_ses *ses, struct cifs_server_iface *iface) SMB2_CLIENT_GUID_SIZE); vol.use_client_guid = true;
- mutex_lock(&ses->session_mutex); + chan_server = cifs_get_tcp_session(&vol);
+ mutex_lock(&ses->session_mutex); + spin_lock(&ses->chan_lock); chan = ses->binding_chan = &ses->chans[ses->chan_count]; - chan->server = cifs_get_tcp_session(&vol); + chan->server = chan_server; if (IS_ERR(chan->server)) { rc = PTR_ERR(chan->server); chan->server = NULL; + spin_unlock(&ses->chan_lock); goto out; } + spin_unlock(&ses->chan_lock); + spin_lock(&cifs_tcp_ses_lock); chan->server->is_channel = true; spin_unlock(&cifs_tcp_ses_lock); @@ -295,8 +318,11 @@ cifs_ses_add_channel(struct cifs_ses *ses, struct cifs_server_iface *iface) * ses to the new server. */
+ spin_lock(&ses->chan_lock); ses->chan_count++; atomic_set(&ses->chan_seq, 0); + spin_unlock(&ses->chan_lock); + out: ses->binding = false; ses->binding_chan = NULL; diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 488893962708..31bec4ce5b58 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -1021,14 +1021,17 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses) if (!ses) return NULL;
+ spin_lock(&ses->chan_lock); if (!ses->binding) { /* round robin */ if (ses->chan_count > 1) { index = (uint)atomic_inc_return(&ses->chan_seq); index %= ses->chan_count; } + spin_unlock(&ses->chan_lock); return ses->chans[index].server; } else { + spin_unlock(&ses->chan_lock); return cifs_ses_server(ses); } }