From 37de5a80e932f828c34abeaae63170d73930dca3 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 6 Nov 2023 14:40:11 +0000 Subject: [PATCH 01/16] cifs: Fix encryption of cleared, but unset rq_iter data buffers Each smb_rqst struct contains two things: an array of kvecs (rq_iov) that contains the protocol data for an RPC op and an iterator (rq_iter) that contains the data payload of an RPC op. When an smb_rqst is allocated rq_iter is it always cleared, but we don't set it up unless we're going to use it. The functions that determines the size of the ciphertext buffer that will be needed to encrypt a request, cifs_get_num_sgs(), assumes that rq_iter is always initialised - and employs user_backed_iter() to check that the iterator isn't user-backed. This used to incidentally work, because ->user_backed was set to false because the iterator has never been initialised, but with commit f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74[1] which changes user_backed_iter() to determine this based on the iterator type insted, a warning is now emitted: WARNING: CPU: 7 PID: 4584 at fs/smb/client/cifsglob.h:2165 smb2_get_aead_req+0x3fc/0x420 [cifs] ... RIP: 0010:smb2_get_aead_req+0x3fc/0x420 [cifs] ... crypt_message+0x33e/0x550 [cifs] smb3_init_transform_rq+0x27d/0x3f0 [cifs] smb_send_rqst+0xc7/0x160 [cifs] compound_send_recv+0x3ca/0x9f0 [cifs] cifs_send_recv+0x25/0x30 [cifs] SMB2_tcon+0x38a/0x820 [cifs] cifs_get_smb_ses+0x69c/0xee0 [cifs] cifs_mount_get_session+0x76/0x1d0 [cifs] dfs_mount_share+0x74/0x9d0 [cifs] cifs_mount+0x6e/0x2e0 [cifs] cifs_smb3_do_mount+0x143/0x300 [cifs] smb3_get_tree+0x15e/0x290 [cifs] vfs_get_tree+0x2d/0xe0 do_new_mount+0x124/0x340 __se_sys_mount+0x143/0x1a0 The problem is that rq_iter was never set, so the type is 0 (ie. ITER_UBUF) which causes user_backed_iter() to return true. The code doesn't malfunction because it checks the size of the iterator - which is 0. Fix cifs_get_num_sgs() to ignore rq_iter if its count is 0, thereby bypassing the warnings. It might be better to explicitly initialise rq_iter to a zero-length ITER_BVEC, say, as it can always be reinitialised later. Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list") Reported-by: Damian Tometzki Closes: https://lore.kernel.org/r/ZUfQo47uo0p2ZsYg@fedora.fritz.box/ Tested-by: Damian Tometzki Cc: stable@vger.kernel.org cc: Eric Biggers cc: linux-cifs@vger.kernel.org cc: linux-fsdevel@vger.kernel.org Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 [1] Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: David Howells Signed-off-by: Steve French --- fs/smb/client/cifsglob.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 02082621d8e0..e55f49e278a2 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -2143,6 +2143,7 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst, unsigned int len, skip; unsigned int nents = 0; unsigned long addr; + size_t data_size; int i, j; /* @@ -2158,17 +2159,21 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst, * rqst[1+].rq_iov[0+] data to be encrypted/decrypted */ for (i = 0; i < num_rqst; i++) { + data_size = iov_iter_count(&rqst[i].rq_iter); + /* We really don't want a mixture of pinned and unpinned pages * in the sglist. It's hard to keep track of which is what. * Instead, we convert to a BVEC-type iterator higher up. */ - if (WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter))) + if (data_size && + WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter))) return -EIO; /* We also don't want to have any extra refs or pins to clean * up in the sglist. */ - if (WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter))) + if (data_size && + WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter))) return -EIO; for (j = 0; j < rqst[i].rq_nvec; j++) { @@ -2184,7 +2189,8 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst, } skip = 0; } - nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX); + if (data_size) + nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX); } nents += DIV_ROUND_UP(offset_in_page(sig) + SMB2_SIGNATURE_SIZE, PAGE_SIZE); return nents; From 43960dc2328e554c4c61b22c47e77e8b1c48d854 Mon Sep 17 00:00:00 2001 From: Steve French Date: Mon, 6 Nov 2023 13:31:45 -0600 Subject: [PATCH 02/16] smb3: minor RDMA cleanup Some minor smbdirect debug cleanup spotted by checkpatch Cc: Long Li Reviewed-by: Bharath SM Signed-off-by: Steve French --- fs/smb/client/cifs_debug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index 6d8804fb6535..0d09bc3ea283 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -746,14 +746,14 @@ static ssize_t name##_write(struct file *file, const char __user *buffer, \ size_t count, loff_t *ppos) \ { \ int rc; \ - rc = kstrtoint_from_user(buffer, count, 10, & name); \ + rc = kstrtoint_from_user(buffer, count, 10, &name); \ if (rc) \ return rc; \ return count; \ } \ static int name##_proc_show(struct seq_file *m, void *v) \ { \ - seq_printf(m, "%d\n", name ); \ + seq_printf(m, "%d\n", name); \ return 0; \ } \ static int name##_open(struct inode *inode, struct file *file) \ From 1bc081b67a79b6e75fae686e98048cea1038ae31 Mon Sep 17 00:00:00 2001 From: Steve French Date: Mon, 6 Nov 2023 22:40:38 -0600 Subject: [PATCH 03/16] smb3: more minor cleanups for session handling routines Some trivial cleanup pointed out by checkpatch Reviewed-by: Bharath SM Signed-off-by: Steve French --- fs/smb/client/sess.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index cd474cf98f30..829c7688a7b7 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -659,8 +659,7 @@ static void ascii_ssetup_strings(char **pbcc_area, struct cifs_ses *ses, if (WARN_ON_ONCE(len < 0)) len = CIFS_MAX_DOMAINNAME_LEN - 1; bcc_ptr += len; - } /* else we will send a null domain name - so the server will default to its own domain */ + } /* else we send a null domain name so server will default to its own domain */ *bcc_ptr = 0; bcc_ptr++; @@ -756,11 +755,14 @@ static void decode_ascii_ssetup(char **pbcc_area, __u16 bleft, if (len > bleft) return; - /* No domain field in LANMAN case. Domain is - returned by old servers in the SMB negprot response */ - /* BB For newer servers which do not support Unicode, - but thus do return domain here we could add parsing - for it later, but it is not very important */ + /* + * No domain field in LANMAN case. Domain is + * returned by old servers in the SMB negprot response + * + * BB For newer servers which do not support Unicode, + * but thus do return domain here, we could add parsing + * for it later, but it is not very important + */ cifs_dbg(FYI, "ascii: bytes left %d\n", bleft); } #endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */ @@ -816,9 +818,12 @@ int decode_ntlmssp_challenge(char *bcc_ptr, int blob_len, ses->ntlmssp->server_flags = server_flags; memcpy(ses->ntlmssp->cryptkey, pblob->Challenge, CIFS_CRYPTO_KEY_SIZE); - /* In particular we can examine sign flags */ - /* BB spec says that if AvId field of MsvAvTimestamp is populated then - we must set the MIC field of the AUTHENTICATE_MESSAGE */ + /* + * In particular we can examine sign flags + * + * BB spec says that if AvId field of MsvAvTimestamp is populated then + * we must set the MIC field of the AUTHENTICATE_MESSAGE + */ tioffset = le32_to_cpu(pblob->TargetInfoArray.BufferOffset); tilen = le16_to_cpu(pblob->TargetInfoArray.Length); From 0c51cc6f2cb0108e7d49805f6e089cd85caab279 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Fri, 13 Oct 2023 09:25:30 +0000 Subject: [PATCH 04/16] cifs: handle cases where a channel is closed So far, SMB multichannel could only scale up, but not scale down the number of channels. In this series of patch, we now allow the client to deal with the case of multichannel disabled on the server when the share is mounted. With that change, we now need the ability to scale down the channels. This change allows the client to deal with cases of missing channels more gracefully. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/cifs_debug.c | 5 +++++ fs/smb/client/cifsglob.h | 1 + fs/smb/client/cifsproto.h | 2 +- fs/smb/client/connect.c | 6 +++++- fs/smb/client/sess.c | 28 ++++++++++++++++++++++++---- fs/smb/client/smb2transport.c | 8 +++++++- 6 files changed, 43 insertions(+), 7 deletions(-) diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index 0d09bc3ea283..772d4226957f 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -136,6 +136,11 @@ cifs_dump_channel(struct seq_file *m, int i, struct cifs_chan *chan) { struct TCP_Server_Info *server = chan->server; + if (!server) { + seq_printf(m, "\n\n\t\tChannel: %d DISABLED", i+1); + return; + } + seq_printf(m, "\n\n\t\tChannel: %d ConnectionId: 0x%llx" "\n\t\tNumber of credits: %d,%d,%d Dialect 0x%x" "\n\t\tTCP status: %d Instance: %d" diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index e55f49e278a2..eae138040edf 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -1050,6 +1050,7 @@ struct cifs_ses { spinlock_t chan_lock; /* ========= begin: protected by chan_lock ======== */ #define CIFS_MAX_CHANNELS 16 +#define CIFS_INVAL_CHAN_INDEX (-1) #define CIFS_ALL_CHANNELS_SET(ses) \ ((1UL << (ses)->chan_count) - 1) #define CIFS_ALL_CHANS_GOOD(ses) \ diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index 890ceddae07e..c1f71c6be7e3 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -616,7 +616,7 @@ bool is_server_using_iface(struct TCP_Server_Info *server, bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface); void cifs_ses_mark_for_reconnect(struct cifs_ses *ses); -unsigned int +int cifs_ses_get_chan_index(struct cifs_ses *ses, struct TCP_Server_Info *server); void diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 1a137b33858a..3ff82f0aa00e 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -173,8 +173,12 @@ cifs_signal_cifsd_for_reconnect(struct TCP_Server_Info *server, list_for_each_entry(ses, &pserver->smb_ses_list, smb_ses_list) { spin_lock(&ses->chan_lock); for (i = 0; i < ses->chan_count; i++) { + if (!ses->chans[i].server) + continue; + spin_lock(&ses->chans[i].server->srv_lock); - ses->chans[i].server->tcpStatus = CifsNeedReconnect; + if (ses->chans[i].server->tcpStatus != CifsExiting) + ses->chans[i].server->tcpStatus = CifsNeedReconnect; spin_unlock(&ses->chans[i].server->srv_lock); } spin_unlock(&ses->chan_lock); diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index 829c7688a7b7..d13a24613710 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -69,7 +69,7 @@ bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface) /* channel helper functions. assumed that chan_lock is held by caller. */ -unsigned int +int cifs_ses_get_chan_index(struct cifs_ses *ses, struct TCP_Server_Info *server) { @@ -85,14 +85,17 @@ cifs_ses_get_chan_index(struct cifs_ses *ses, cifs_dbg(VFS, "unable to get chan index for server: 0x%llx", server->conn_id); WARN_ON(1); - return 0; + return CIFS_INVAL_CHAN_INDEX; } void cifs_chan_set_in_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { - unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + int chan_index = cifs_ses_get_chan_index(ses, server); + + if (chan_index == CIFS_INVAL_CHAN_INDEX) + return; ses->chans[chan_index].in_reconnect = true; } @@ -102,6 +105,8 @@ cifs_chan_clear_in_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) + return; ses->chans[chan_index].in_reconnect = false; } @@ -111,6 +116,8 @@ cifs_chan_in_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) + return true; /* err on the safer side */ return CIFS_CHAN_IN_RECONNECT(ses, chan_index); } @@ -120,6 +127,8 @@ cifs_chan_set_need_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) + return; set_bit(chan_index, &ses->chans_need_reconnect); cifs_dbg(FYI, "Set reconnect bitmask for chan %u; now 0x%lx\n", @@ -131,6 +140,8 @@ cifs_chan_clear_need_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) + return; clear_bit(chan_index, &ses->chans_need_reconnect); cifs_dbg(FYI, "Cleared reconnect bitmask for chan %u; now 0x%lx\n", @@ -142,6 +153,8 @@ cifs_chan_needs_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) + return true; /* err on the safer side */ return CIFS_CHAN_NEEDS_RECONNECT(ses, chan_index); } @@ -151,6 +164,8 @@ cifs_chan_is_iface_active(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) + return true; /* err on the safer side */ return ses->chans[chan_index].iface && ses->chans[chan_index].iface->is_active; @@ -269,7 +284,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) spin_lock(&ses->chan_lock); chan_index = cifs_ses_get_chan_index(ses, server); - if (!chan_index) { + if (chan_index == CIFS_INVAL_CHAN_INDEX) { spin_unlock(&ses->chan_lock); return 0; } @@ -319,6 +334,11 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) spin_lock(&ses->chan_lock); chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) { + spin_unlock(&ses->chan_lock); + return 0; + } + ses->chans[chan_index].iface = iface; /* No iface is found. if secondary chan, drop connection */ diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c index 23c50ed7d4b5..84ea67301303 100644 --- a/fs/smb/client/smb2transport.c +++ b/fs/smb/client/smb2transport.c @@ -413,7 +413,13 @@ generate_smb3signingkey(struct cifs_ses *ses, ses->ses_status == SES_GOOD); chan_index = cifs_ses_get_chan_index(ses, server); - /* TODO: introduce ref counting for channels when the can be freed */ + if (chan_index == CIFS_INVAL_CHAN_INDEX) { + spin_unlock(&ses->chan_lock); + spin_unlock(&ses->ses_lock); + + return -EINVAL; + } + spin_unlock(&ses->chan_lock); spin_unlock(&ses->ses_lock); From a6d8fb54a515f0546ffdb7870102b1238917e567 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Mon, 26 Dec 2022 11:24:56 +0000 Subject: [PATCH 05/16] cifs: distribute channels across interfaces based on speed Today, if the server interfaces RSS capable, we simply choose the fastest interface to setup a channel. This is not a scalable approach, and does not make a lot of attempt to distribute the connections. This change does a weighted distribution of channels across all the available server interfaces, where the weight is a function of the advertised interface speed. Also make sure that we don't mix rdma and non-rdma for channels. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/cifs_debug.c | 16 ++++++++ fs/smb/client/cifsglob.h | 2 + fs/smb/client/sess.c | 84 +++++++++++++++++++++++++++++++------- 3 files changed, 88 insertions(+), 14 deletions(-) diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index 772d4226957f..5596c9f30ccb 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -284,6 +284,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) struct cifs_ses *ses; struct cifs_tcon *tcon; struct cifs_server_iface *iface; + size_t iface_weight = 0, iface_min_speed = 0; + struct cifs_server_iface *last_iface = NULL; int c, i, j; seq_puts(m, @@ -549,11 +551,25 @@ skip_rdma: "\tLast updated: %lu seconds ago", ses->iface_count, (jiffies - ses->iface_last_update) / HZ); + + last_iface = list_last_entry(&ses->iface_list, + struct cifs_server_iface, + iface_head); + iface_min_speed = last_iface->speed; + j = 0; list_for_each_entry(iface, &ses->iface_list, iface_head) { seq_printf(m, "\n\t%d)", ++j); cifs_dump_iface(m, iface); + + iface_weight = iface->speed / iface_min_speed; + seq_printf(m, "\t\tWeight (cur,total): (%zu,%zu)" + "\n\t\tAllocated channels: %u\n", + iface->weight_fulfilled, + iface_weight, + iface->num_channels); + if (is_ses_using_iface(ses, iface)) seq_puts(m, "\t\t[CONNECTED]\n"); } diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index eae138040edf..30763c68cc39 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -969,6 +969,8 @@ struct cifs_server_iface { struct list_head iface_head; struct kref refcount; size_t speed; + size_t weight_fulfilled; + unsigned int num_channels; unsigned int rdma_capable : 1; unsigned int rss_capable : 1; unsigned int is_active : 1; /* unset if non existent */ diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index d13a24613710..336b64d93e41 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -179,7 +179,9 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses) int left; int rc = 0; int tries = 0; + size_t iface_weight = 0, iface_min_speed = 0; struct cifs_server_iface *iface = NULL, *niface = NULL; + struct cifs_server_iface *last_iface = NULL; spin_lock(&ses->chan_lock); @@ -207,21 +209,11 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses) } spin_unlock(&ses->chan_lock); - /* - * Keep connecting to same, fastest, iface for all channels as - * long as its RSS. Try next fastest one if not RSS or channel - * creation fails. - */ - spin_lock(&ses->iface_lock); - iface = list_first_entry(&ses->iface_list, struct cifs_server_iface, - iface_head); - spin_unlock(&ses->iface_lock); - while (left > 0) { tries++; if (tries > 3*ses->chan_max) { - cifs_dbg(FYI, "too many channel open attempts (%d channels left to open)\n", + cifs_dbg(VFS, "too many channel open attempts (%d channels left to open)\n", left); break; } @@ -229,17 +221,35 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses) spin_lock(&ses->iface_lock); if (!ses->iface_count) { spin_unlock(&ses->iface_lock); + cifs_dbg(VFS, "server %s does not advertise interfaces\n", + ses->server->hostname); break; } + if (!iface) + iface = list_first_entry(&ses->iface_list, struct cifs_server_iface, + iface_head); + last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface, + iface_head); + iface_min_speed = last_iface->speed; + list_for_each_entry_safe_from(iface, niface, &ses->iface_list, iface_head) { + /* do not mix rdma and non-rdma interfaces */ + if (iface->rdma_capable != ses->server->rdma) + continue; + /* skip ifaces that are unusable */ if (!iface->is_active || (is_ses_using_iface(ses, iface) && - !iface->rss_capable)) { + !iface->rss_capable)) + continue; + + /* check if we already allocated enough channels */ + iface_weight = iface->speed / iface_min_speed; + + if (iface->weight_fulfilled >= iface_weight) continue; - } /* take ref before unlock */ kref_get(&iface->refcount); @@ -256,10 +266,21 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses) continue; } - cifs_dbg(FYI, "successfully opened new channel on iface:%pIS\n", + iface->num_channels++; + iface->weight_fulfilled++; + cifs_dbg(VFS, "successfully opened new channel on iface:%pIS\n", &iface->sockaddr); break; } + + /* reached end of list. reset weight_fulfilled and start over */ + if (list_entry_is_head(iface, &ses->iface_list, iface_head)) { + list_for_each_entry(iface, &ses->iface_list, iface_head) + iface->weight_fulfilled = 0; + spin_unlock(&ses->iface_lock); + iface = NULL; + continue; + } spin_unlock(&ses->iface_lock); left--; @@ -278,8 +299,10 @@ int cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index; + size_t iface_weight = 0, iface_min_speed = 0; struct cifs_server_iface *iface = NULL; struct cifs_server_iface *old_iface = NULL; + struct cifs_server_iface *last_iface = NULL; int rc = 0; spin_lock(&ses->chan_lock); @@ -299,13 +322,34 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) spin_unlock(&ses->chan_lock); spin_lock(&ses->iface_lock); + if (!ses->iface_count) { + spin_unlock(&ses->iface_lock); + cifs_dbg(VFS, "server %s does not advertise interfaces\n", ses->server->hostname); + return 0; + } + + last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface, + iface_head); + iface_min_speed = last_iface->speed; + /* then look for a new one */ list_for_each_entry(iface, &ses->iface_list, iface_head) { + /* do not mix rdma and non-rdma interfaces */ + if (iface->rdma_capable != server->rdma) + continue; + if (!iface->is_active || (is_ses_using_iface(ses, iface) && !iface->rss_capable)) { continue; } + + /* check if we already allocated enough channels */ + iface_weight = iface->speed / iface_min_speed; + + if (iface->weight_fulfilled >= iface_weight) + continue; + kref_get(&iface->refcount); break; } @@ -321,10 +365,22 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) cifs_dbg(FYI, "replacing iface: %pIS with %pIS\n", &old_iface->sockaddr, &iface->sockaddr); + + old_iface->num_channels--; + if (old_iface->weight_fulfilled) + old_iface->weight_fulfilled--; + iface->num_channels++; + iface->weight_fulfilled++; + kref_put(&old_iface->refcount, release_iface); } else if (old_iface) { cifs_dbg(FYI, "releasing ref to iface: %pIS\n", &old_iface->sockaddr); + + old_iface->num_channels--; + if (old_iface->weight_fulfilled) + old_iface->weight_fulfilled--; + kref_put(&old_iface->refcount, release_iface); } else { WARN_ON(!iface); From fa1d0508bdd4a68c5e40f85f635712af8c12f180 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Tue, 14 Mar 2023 11:14:58 +0000 Subject: [PATCH 06/16] cifs: account for primary channel in the interface list The refcounting of server interfaces should account for the primary channel too. Although this is not strictly necessary, doing so will account for the primary channel in DebugData. Cc: stable@vger.kernel.org Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/sess.c | 28 ++++++++++++++++++++++++++++ fs/smb/client/smb2ops.c | 6 ++++++ 2 files changed, 34 insertions(+) diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index 336b64d93e41..e716d046fb5f 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -303,6 +303,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) struct cifs_server_iface *iface = NULL; struct cifs_server_iface *old_iface = NULL; struct cifs_server_iface *last_iface = NULL; + struct sockaddr_storage ss; int rc = 0; spin_lock(&ses->chan_lock); @@ -321,6 +322,10 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) } spin_unlock(&ses->chan_lock); + spin_lock(&server->srv_lock); + ss = server->dstaddr; + spin_unlock(&server->srv_lock); + spin_lock(&ses->iface_lock); if (!ses->iface_count) { spin_unlock(&ses->iface_lock); @@ -334,6 +339,16 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) /* then look for a new one */ list_for_each_entry(iface, &ses->iface_list, iface_head) { + if (!chan_index) { + /* if we're trying to get the updated iface for primary channel */ + if (!cifs_match_ipaddr((struct sockaddr *) &ss, + (struct sockaddr *) &iface->sockaddr)) + continue; + + kref_get(&iface->refcount); + break; + } + /* do not mix rdma and non-rdma interfaces */ if (iface->rdma_capable != server->rdma) continue; @@ -360,6 +375,13 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) cifs_dbg(FYI, "unable to find a suitable iface\n"); } + if (!chan_index && !iface) { + cifs_dbg(FYI, "unable to get the interface matching: %pIS\n", + &ss); + spin_unlock(&ses->iface_lock); + return 0; + } + /* now drop the ref to the current iface */ if (old_iface && iface) { cifs_dbg(FYI, "replacing iface: %pIS with %pIS\n", @@ -382,6 +404,12 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) old_iface->weight_fulfilled--; kref_put(&old_iface->refcount, release_iface); + } else if (!chan_index) { + /* special case: update interface for primary channel */ + cifs_dbg(FYI, "referencing primary channel iface: %pIS\n", + &iface->sockaddr); + iface->num_channels++; + iface->weight_fulfilled++; } else { WARN_ON(!iface); cifs_dbg(FYI, "adding new iface: %pIS\n", &iface->sockaddr); diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 601e7a187f87..a959ed2c9b22 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -756,6 +756,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_ unsigned int ret_data_len = 0; struct network_interface_info_ioctl_rsp *out_buf = NULL; struct cifs_ses *ses = tcon->ses; + struct TCP_Server_Info *pserver; /* do not query too frequently */ if (ses->iface_last_update && @@ -780,6 +781,11 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_ if (rc) goto out; + /* check if iface is still active */ + pserver = ses->chans[0].server; + if (pserver && !cifs_chan_is_iface_active(ses, pserver)) + cifs_chan_update_iface(ses, pserver); + out: kfree(out_buf); return rc; From 9599d59eb8fc0c0fd9480c4f22901533d08965ee Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Mon, 6 Nov 2023 16:22:11 +0000 Subject: [PATCH 07/16] cifs: do not pass cifs_sb when trying to add channels The only reason why cifs_sb gets passed today to cifs_try_adding_channels is to pass the local_nls field for the new channels and binding session. However, the ses struct already has local_nls field that is setup during the first cifs_setup_session. So there is no need to pass cifs_sb. This change removes cifs_sb from the arg list for this and the functions that it calls and uses ses->local_nls instead. Cc: stable@vger.kernel.org Signed-off-by: Shyam Prasad N Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/cifsproto.h | 2 +- fs/smb/client/connect.c | 2 +- fs/smb/client/sess.c | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index c1f71c6be7e3..eed8dbb6b2fb 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -610,7 +610,7 @@ void cifs_free_hash(struct shash_desc **sdesc); struct cifs_chan * cifs_ses_find_chan(struct cifs_ses *ses, struct TCP_Server_Info *server); -int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses); +int cifs_try_adding_channels(struct cifs_ses *ses); bool is_server_using_iface(struct TCP_Server_Info *server, struct cifs_server_iface *iface); bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface); diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 3ff82f0aa00e..947e3c362beb 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -3564,7 +3564,7 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx) ctx->prepath = NULL; out: - cifs_try_adding_channels(cifs_sb, mnt_ctx.ses); + cifs_try_adding_channels(mnt_ctx.ses); rc = mount_setup_tlink(cifs_sb, mnt_ctx.ses, mnt_ctx.tcon); if (rc) goto error; diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index e716d046fb5f..fe45ccfdc802 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -24,7 +24,7 @@ #include "fs_context.h" static int -cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses, +cifs_ses_add_channel(struct cifs_ses *ses, struct cifs_server_iface *iface); bool @@ -172,7 +172,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses, } /* returns number of channels added */ -int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses) +int cifs_try_adding_channels(struct cifs_ses *ses) { struct TCP_Server_Info *server = ses->server; int old_chan_count, new_chan_count; @@ -255,7 +255,7 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses) kref_get(&iface->refcount); spin_unlock(&ses->iface_lock); - rc = cifs_ses_add_channel(cifs_sb, ses, iface); + rc = cifs_ses_add_channel(ses, iface); spin_lock(&ses->iface_lock); if (rc) { @@ -458,7 +458,7 @@ cifs_ses_find_chan(struct cifs_ses *ses, struct TCP_Server_Info *server) } static int -cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses, +cifs_ses_add_channel(struct cifs_ses *ses, struct cifs_server_iface *iface) { struct TCP_Server_Info *chan_server; @@ -537,7 +537,7 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses, * This will be used for encoding/decoding user/domain/pw * during sess setup auth. */ - ctx->local_nls = cifs_sb->local_nls; + ctx->local_nls = ses->local_nls; /* Use RDMA if possible */ ctx->rdma = iface->rdma_capable; @@ -583,7 +583,7 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses, rc = cifs_negotiate_protocol(xid, ses, chan->server); if (!rc) - rc = cifs_setup_session(xid, ses, chan->server, cifs_sb->local_nls); + rc = cifs_setup_session(xid, ses, chan->server, ses->local_nls); mutex_unlock(&ses->session_mutex); From 19a4b9d6c372cab6a3b2c9a061a236136fe95274 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Fri, 13 Oct 2023 11:43:09 +0000 Subject: [PATCH 08/16] cifs: reconnect work should have reference on server struct The delayed work for reconnect takes server struct as a parameter. But it does so without holding a ref to it. Normally, this may not show a problem as the reconnect work is only cancelled on umount. However, since we now plan to support scaling down of channels, and the scale down can happen from reconnect work itself, we need to fix it. This change takes a reference on the server struct before it is passed to the delayed work. And drops the reference in the delayed work itself. Or if the delayed work is successfully cancelled, by the process that cancels it. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/connect.c | 27 +++++++++++++++++++++------ fs/smb/client/smb2pdu.c | 23 +++++++++++++---------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 947e3c362beb..b514b41cc9f0 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -389,7 +389,13 @@ static int __cifs_reconnect(struct TCP_Server_Info *server, spin_unlock(&server->srv_lock); cifs_swn_reset_server_dstaddr(server); cifs_server_unlock(server); - mod_delayed_work(cifsiod_wq, &server->reconnect, 0); + + /* increase ref count which reconnect work will drop */ + spin_lock(&cifs_tcp_ses_lock); + server->srv_count++; + spin_unlock(&cifs_tcp_ses_lock); + if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0)) + cifs_put_tcp_session(server, false); } } while (server->tcpStatus == CifsNeedReconnect); @@ -519,7 +525,13 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server) spin_unlock(&server->srv_lock); cifs_swn_reset_server_dstaddr(server); cifs_server_unlock(server); - mod_delayed_work(cifsiod_wq, &server->reconnect, 0); + + /* increase ref count which reconnect work will drop */ + spin_lock(&cifs_tcp_ses_lock); + server->srv_count++; + spin_unlock(&cifs_tcp_ses_lock); + if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0)) + cifs_put_tcp_session(server, false); } while (server->tcpStatus == CifsNeedReconnect); mutex_lock(&server->refpath_lock); @@ -1601,16 +1613,19 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect) cancel_delayed_work_sync(&server->echo); - if (from_reconnect) + if (from_reconnect) { /* * Avoid deadlock here: reconnect work calls * cifs_put_tcp_session() at its end. Need to be sure * that reconnect work does nothing with server pointer after * that step. */ - cancel_delayed_work(&server->reconnect); - else - cancel_delayed_work_sync(&server->reconnect); + if (cancel_delayed_work(&server->reconnect)) + cifs_put_tcp_session(server, from_reconnect); + } else { + if (cancel_delayed_work_sync(&server->reconnect)) + cifs_put_tcp_session(server, from_reconnect); + } spin_lock(&server->srv_lock); server->tcpStatus = CifsExiting; diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index c75a80bb6d9e..b7665155f4e2 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -3852,12 +3852,6 @@ void smb2_reconnect_server(struct work_struct *work) } spin_unlock(&ses->chan_lock); } - /* - * Get the reference to server struct to be sure that the last call of - * cifs_put_tcon() in the loop below won't release the server pointer. - */ - if (tcon_exist || ses_exist) - server->srv_count++; spin_unlock(&cifs_tcp_ses_lock); @@ -3905,13 +3899,17 @@ void smb2_reconnect_server(struct work_struct *work) done: cifs_dbg(FYI, "Reconnecting tcons and channels finished\n"); - if (resched) + if (resched) { queue_delayed_work(cifsiod_wq, &server->reconnect, 2 * HZ); + mutex_unlock(&pserver->reconnect_mutex); + + /* no need to put tcp session as we're retrying */ + return; + } mutex_unlock(&pserver->reconnect_mutex); /* now we can safely release srv struct */ - if (tcon_exist || ses_exist) - cifs_put_tcp_session(server, 1); + cifs_put_tcp_session(server, true); } int @@ -3931,7 +3929,12 @@ SMB2_echo(struct TCP_Server_Info *server) server->ops->need_neg(server)) { spin_unlock(&server->srv_lock); /* No need to send echo on newly established connections */ - mod_delayed_work(cifsiod_wq, &server->reconnect, 0); + spin_lock(&cifs_tcp_ses_lock); + server->srv_count++; + spin_unlock(&cifs_tcp_ses_lock); + if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0)) + cifs_put_tcp_session(server, false); + return rc; } spin_unlock(&server->srv_lock); From f72d96507640835726d4f5ba26c1c11acbe1bc97 Mon Sep 17 00:00:00 2001 From: Steve French Date: Mon, 6 Nov 2023 15:37:03 -0600 Subject: [PATCH 09/16] smb3: minor cleanup of session handling code Minor cleanup of style issues found by checkpatch Reviewed-by: Bharath SM Signed-off-by: Steve French --- fs/smb/client/sess.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index fe45ccfdc802..4e4cc0cd5206 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -105,6 +105,7 @@ cifs_chan_clear_in_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) return; @@ -116,6 +117,7 @@ cifs_chan_in_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) return true; /* err on the safer side */ @@ -127,6 +129,7 @@ cifs_chan_set_need_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) return; @@ -140,6 +143,7 @@ cifs_chan_clear_need_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) return; @@ -153,6 +157,7 @@ cifs_chan_needs_reconnect(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) return true; /* err on the safer side */ @@ -164,6 +169,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses, struct TCP_Server_Info *server) { unsigned int chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) return true; /* err on the safer side */ @@ -639,8 +645,7 @@ static __u32 cifs_ssetup_hdr(struct cifs_ses *ses, /* Now no need to set SMBFLG_CASELESS or obsolete CANONICAL PATH */ - /* BB verify whether signing required on neg or just on auth frame - (and NTLM case) */ + /* BB verify whether signing required on neg or just auth frame (and NTLM case) */ capabilities = CAP_LARGE_FILES | CAP_NT_SMBS | CAP_LEVEL_II_OPLOCKS | CAP_LARGE_WRITE_X | CAP_LARGE_READ_X; @@ -697,8 +702,10 @@ static void unicode_domain_string(char **pbcc_area, struct cifs_ses *ses, /* copy domain */ if (ses->domainName == NULL) { - /* Sending null domain better than using a bogus domain name (as - we did briefly in 2.6.18) since server will use its default */ + /* + * Sending null domain better than using a bogus domain name (as + * we did briefly in 2.6.18) since server will use its default + */ *bcc_ptr = 0; *(bcc_ptr+1) = 0; bytes_ret = 0; @@ -717,8 +724,7 @@ static void unicode_ssetup_strings(char **pbcc_area, struct cifs_ses *ses, char *bcc_ptr = *pbcc_area; int bytes_ret = 0; - /* BB FIXME add check that strings total less - than 335 or will need to send them as arrays */ + /* BB FIXME add check that strings less than 335 or will need to send as arrays */ /* copy user */ if (ses->user_name == NULL) { From 5923d6686a100c2b4cabd4c2ca9d5a12579c7614 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 7 Nov 2023 21:38:13 -0600 Subject: [PATCH 10/16] smb3: fix caching of ctime on setxattr Fixes xfstest generic/728 which had been failing due to incorrect ctime after setxattr and removexattr Update ctime on successful set of xattr Cc: stable@vger.kernel.org Signed-off-by: Steve French --- fs/smb/client/xattr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/xattr.c b/fs/smb/client/xattr.c index ac199160bce6..6780aa3e98a1 100644 --- a/fs/smb/client/xattr.c +++ b/fs/smb/client/xattr.c @@ -150,10 +150,13 @@ static int cifs_xattr_set(const struct xattr_handler *handler, if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR) goto out; - if (pTcon->ses->server->ops->set_EA) + if (pTcon->ses->server->ops->set_EA) { rc = pTcon->ses->server->ops->set_EA(xid, pTcon, full_path, name, value, (__u16)size, cifs_sb->local_nls, cifs_sb); + if (rc == 0) + inode_set_ctime_current(inode); + } break; case XATTR_CIFS_ACL: From 5e2fd17f434d2fed78efb123e2fc6711e4f598f1 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Thu, 9 Nov 2023 12:01:48 -0300 Subject: [PATCH 11/16] smb: client: fix mount when dns_resolver key is not available There was a wrong assumption that with CONFIG_CIFS_DFS_UPCALL=y there would always be a dns_resolver key set up so we could unconditionally upcall to resolve UNC hostname rather than using the value provided by mount(2). Only require it when performing automount of junctions within a DFS share so users that don't have dns_resolver key still can mount their regular shares with server hostname resolved by mount.cifs(8). Fixes: 348a04a8d113 ("smb: client: get rid of dfs code dep in namespace.c") Cc: stable@vger.kernel.org Tested-by: Eduard Bachmakov Reported-by: Eduard Bachmakov Closes: https://lore.kernel.org/all/CADCRUiNvZuiUZ0VGZZO9HRyPyw6x92kiA7o7Q4tsX5FkZqUkKg@mail.gmail.com/ Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/dfs.c | 18 +++++++++++++----- fs/smb/client/fs_context.h | 1 + fs/smb/client/namespace.c | 17 +++++++++++++++-- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/fs/smb/client/dfs.c b/fs/smb/client/dfs.c index 81b84151450d..a8a1d386da65 100644 --- a/fs/smb/client/dfs.c +++ b/fs/smb/client/dfs.c @@ -263,15 +263,23 @@ out: return rc; } -/* Resolve UNC hostname in @ctx->source and set ip addr in @ctx->dstaddr */ +/* + * If @ctx->dfs_automount, then update @ctx->dstaddr earlier with the DFS root + * server from where we'll start following any referrals. Otherwise rely on the + * value provided by mount(2) as the user might not have dns_resolver key set up + * and therefore failing to upcall to resolve UNC hostname under @ctx->source. + */ static int update_fs_context_dstaddr(struct smb3_fs_context *ctx) { struct sockaddr *addr = (struct sockaddr *)&ctx->dstaddr; - int rc; + int rc = 0; - rc = dns_resolve_server_name_to_ip(ctx->source, addr, NULL); - if (!rc) - cifs_set_port(addr, ctx->port); + if (!ctx->nodfs && ctx->dfs_automount) { + rc = dns_resolve_server_name_to_ip(ctx->source, addr, NULL); + if (!rc) + cifs_set_port(addr, ctx->port); + ctx->dfs_automount = false; + } return rc; } diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h index 9d8d34af0211..cf46916286d0 100644 --- a/fs/smb/client/fs_context.h +++ b/fs/smb/client/fs_context.h @@ -268,6 +268,7 @@ struct smb3_fs_context { bool witness:1; /* use witness protocol */ char *leaf_fullpath; struct cifs_ses *dfs_root_ses; + bool dfs_automount:1; /* set for dfs automount only */ }; extern const struct fs_parameter_spec smb3_fs_parameters[]; diff --git a/fs/smb/client/namespace.c b/fs/smb/client/namespace.c index c8f5ed8a69f1..a6968573b775 100644 --- a/fs/smb/client/namespace.c +++ b/fs/smb/client/namespace.c @@ -117,6 +117,18 @@ cifs_build_devname(char *nodename, const char *prepath) return dev; } +static bool is_dfs_mount(struct dentry *dentry) +{ + struct cifs_sb_info *cifs_sb = CIFS_SB(dentry->d_sb); + struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); + bool ret; + + spin_lock(&tcon->tc_lock); + ret = !!tcon->origin_fullpath; + spin_unlock(&tcon->tc_lock); + return ret; +} + /* Return full path out of a dentry set for automount */ static char *automount_fullpath(struct dentry *dentry, void *page) { @@ -212,8 +224,9 @@ static struct vfsmount *cifs_do_automount(struct path *path) ctx->source = NULL; goto out; } - cifs_dbg(FYI, "%s: ctx: source=%s UNC=%s prepath=%s\n", - __func__, ctx->source, ctx->UNC, ctx->prepath); + ctx->dfs_automount = is_dfs_mount(mntpt); + cifs_dbg(FYI, "%s: ctx: source=%s UNC=%s prepath=%s dfs_automount=%d\n", + __func__, ctx->source, ctx->UNC, ctx->prepath, ctx->dfs_automount); mnt = fc_mount(fc); out: From de4eceab578ead12a71e5b5588a57e142bbe8ceb Mon Sep 17 00:00:00 2001 From: Steve French Date: Thu, 9 Nov 2023 15:28:12 -0600 Subject: [PATCH 12/16] smb3: allow dumping session and tcon id to improve stats analysis and debugging When multiple mounts are to the same share from the same client it was not possible to determine which section of /proc/fs/cifs/Stats (and DebugData) correspond to that mount. In some recent examples this turned out to be a significant problem when trying to analyze performance data - since there are many cases where unless we know the tree id and session id we can't figure out which stats (e.g. number of SMB3.1.1 requests by type, the total time they take, which is slowest, how many fail etc.) apply to which mount. The only existing loosely related ioctl CIFS_IOC_GET_MNT_INFO does not return the information needed to uniquely identify which tcon is which mount although it does return various flags and device info. Add a cifs.ko ioctl CIFS_IOC_GET_TCON_INFO (0x800ccf0c) to return tid, session id, tree connect count. Cc: stable@vger.kernel.org Reviewed-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/cifs_ioctl.h | 6 ++++++ fs/smb/client/ioctl.c | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/fs/smb/client/cifs_ioctl.h b/fs/smb/client/cifs_ioctl.h index 332588e77c31..26327442e383 100644 --- a/fs/smb/client/cifs_ioctl.h +++ b/fs/smb/client/cifs_ioctl.h @@ -26,6 +26,11 @@ struct smb_mnt_fs_info { __u64 cifs_posix_caps; } __packed; +struct smb_mnt_tcon_info { + __u32 tid; + __u64 session_id; +} __packed; + struct smb_snapshot_array { __u32 number_of_snapshots; __u32 number_of_snapshots_returned; @@ -108,6 +113,7 @@ struct smb3_notify_info { #define CIFS_IOC_NOTIFY _IOW(CIFS_IOCTL_MAGIC, 9, struct smb3_notify) #define CIFS_DUMP_FULL_KEY _IOWR(CIFS_IOCTL_MAGIC, 10, struct smb3_full_key_debug_info) #define CIFS_IOC_NOTIFY_INFO _IOWR(CIFS_IOCTL_MAGIC, 11, struct smb3_notify_info) +#define CIFS_IOC_GET_TCON_INFO _IOR(CIFS_IOCTL_MAGIC, 12, struct smb_mnt_tcon_info) #define CIFS_IOC_SHUTDOWN _IOR('X', 125, __u32) /* diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c index f7160003e0ed..73ededa8eba5 100644 --- a/fs/smb/client/ioctl.c +++ b/fs/smb/client/ioctl.c @@ -117,6 +117,20 @@ out_drop_write: return rc; } +static long smb_mnt_get_tcon_info(struct cifs_tcon *tcon, void __user *arg) +{ + int rc = 0; + struct smb_mnt_tcon_info tcon_inf; + + tcon_inf.tid = tcon->tid; + tcon_inf.session_id = tcon->ses->Suid; + + if (copy_to_user(arg, &tcon_inf, sizeof(struct smb_mnt_tcon_info))) + rc = -EFAULT; + + return rc; +} + static long smb_mnt_get_fsinfo(unsigned int xid, struct cifs_tcon *tcon, void __user *arg) { @@ -414,6 +428,17 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg) tcon = tlink_tcon(pSMBFile->tlink); rc = smb_mnt_get_fsinfo(xid, tcon, (void __user *)arg); break; + case CIFS_IOC_GET_TCON_INFO: + cifs_sb = CIFS_SB(inode->i_sb); + tlink = cifs_sb_tlink(cifs_sb); + if (IS_ERR(tlink)) { + rc = PTR_ERR(tlink); + break; + } + tcon = tlink_tcon(tlink); + rc = smb_mnt_get_tcon_info(tcon, (void __user *)arg); + cifs_put_tlink(tlink); + break; case CIFS_ENUMERATE_SNAPSHOTS: if (pSMBFile == NULL) break; From 784e0e20b4c97c270b2892f677d3fad658e2c1d5 Mon Sep 17 00:00:00 2001 From: Steve French Date: Fri, 10 Nov 2023 01:24:16 -0600 Subject: [PATCH 13/16] Missing field not being returned in ioctl CIFS_IOC_GET_MNT_INFO The tcon_flags field was always being set to zero in the information about the mount returned by the ioctl CIFS_IOC_GET_MNT_INFO instead of being set to the value of the Flags field in the tree connection structure as intended. Reviewed-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/ioctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c index 73ededa8eba5..e2f92c21fff5 100644 --- a/fs/smb/client/ioctl.c +++ b/fs/smb/client/ioctl.c @@ -143,6 +143,7 @@ static long smb_mnt_get_fsinfo(unsigned int xid, struct cifs_tcon *tcon, fsinf->version = 1; fsinf->protocol_id = tcon->ses->server->vals->protocol_id; + fsinf->tcon_flags = tcon->Flags; fsinf->device_characteristics = le32_to_cpu(tcon->fsDevInfo.DeviceCharacteristics); fsinf->device_type = le32_to_cpu(tcon->fsDevInfo.DeviceType); From 705fc522fe9d58848c253ee0948567060f36e2a7 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Fri, 13 Oct 2023 11:33:21 +0000 Subject: [PATCH 14/16] cifs: handle when server starts supporting multichannel When the user mounts with multichannel option, but the server does not support it, there can be a time in future where it can be supported. With this change, such a case is handled. Signed-off-by: Shyam Prasad N --- fs/smb/client/cifsproto.h | 1 + fs/smb/client/connect.c | 3 +++ fs/smb/client/smb2pdu.c | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index eed8dbb6b2fb..00ad41633e26 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -132,6 +132,7 @@ extern int SendReceiveBlockingLock(const unsigned int xid, struct smb_hdr *in_buf, struct smb_hdr *out_buf, int *bytes_returned); + void cifs_signal_cifsd_for_reconnect(struct TCP_Server_Info *server, bool all_channels); diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index b514b41cc9f0..1490547ec493 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -132,6 +132,9 @@ static void smb2_query_server_interfaces(struct work_struct *work) free_xid(xid); if (rc) { + if (rc == -EOPNOTSUPP) + return; + cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n", __func__, rc); } diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index b7665155f4e2..0d4351e43069 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -163,6 +163,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, int rc = 0; struct nls_table *nls_codepage = NULL; struct cifs_ses *ses; + int xid; /* * SMB2s NegProt, SessSetup, Logoff do not have tcon yet so @@ -307,17 +308,44 @@ skip_sess_setup: tcon->need_reopen_files = true; rc = cifs_tree_connect(0, tcon, nls_codepage); - mutex_unlock(&ses->session_mutex); cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc); if (rc) { /* If sess reconnected but tcon didn't, something strange ... */ + mutex_unlock(&ses->session_mutex); cifs_dbg(VFS, "reconnect tcon failed rc = %d\n", rc); goto out; } + if (!rc && + (server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { + mutex_unlock(&ses->session_mutex); + + /* + * query server network interfaces, in case they change + */ + xid = get_xid(); + rc = SMB3_request_interfaces(xid, tcon, false); + free_xid(xid); + + if (rc) + cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n", + __func__, rc); + + if (ses->chan_max > ses->chan_count && + !SERVER_IS_CHAN(server)) { + if (ses->chan_count == 1) + cifs_server_dbg(VFS, "supports multichannel now\n"); + + cifs_try_adding_channels(ses); + } + } else { + mutex_unlock(&ses->session_mutex); + } + if (smb2_command != SMB2_INTERNAL_CMD) - mod_delayed_work(cifsiod_wq, &server->reconnect, 0); + if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0)) + cifs_put_tcp_session(server, false); atomic_inc(&tconInfoReconnectCount); out: From ee1d21794e55ab76505745d24101331552182002 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Fri, 13 Oct 2023 11:40:09 +0000 Subject: [PATCH 15/16] cifs: handle when server stops supporting multichannel When a server stops supporting multichannel, we will keep attempting reconnects to the secondary channels today. Avoid this by freeing extra channels when negotiate returns no multichannel support. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/cifsglob.h | 1 + fs/smb/client/cifsproto.h | 2 ++ fs/smb/client/connect.c | 10 ++++++ fs/smb/client/sess.c | 64 ++++++++++++++++++++++++++++----- fs/smb/client/smb2pdu.c | 76 ++++++++++++++++++++++++++++++++++++++- fs/smb/client/transport.c | 2 +- 6 files changed, 145 insertions(+), 10 deletions(-) diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 30763c68cc39..6ffbd81bd109 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -650,6 +650,7 @@ struct TCP_Server_Info { bool noautotune; /* do not autotune send buf sizes */ bool nosharesock; bool tcp_nodelay; + bool terminate; unsigned int credits; /* send no more requests at once */ unsigned int max_credits; /* can override large 32000 default at mnt */ unsigned int in_flight; /* number of requests on the wire to server */ diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index 00ad41633e26..d87e2c26cce2 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -641,6 +641,8 @@ cifs_chan_needs_reconnect(struct cifs_ses *ses, bool cifs_chan_is_iface_active(struct cifs_ses *ses, struct TCP_Server_Info *server); +void +cifs_disable_secondary_channels(struct cifs_ses *ses); int cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server); int diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 1490547ec493..57c2a7df3457 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -219,6 +219,14 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server, spin_lock(&cifs_tcp_ses_lock); list_for_each_entry_safe(ses, nses, &pserver->smb_ses_list, smb_ses_list) { + /* + * if channel has been marked for termination, nothing to do + * for the channel. in fact, we cannot find the channel for the + * server. So safe to exit here + */ + if (server->terminate) + break; + /* check if iface is still active */ if (!cifs_chan_is_iface_active(ses, server)) cifs_chan_update_iface(ses, server); @@ -253,6 +261,8 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server, spin_lock(&tcon->tc_lock); tcon->status = TID_NEED_RECON; spin_unlock(&tcon->tc_lock); + + cancel_delayed_work(&tcon->query_interfaces); } if (ses->tcon_ipc) { ses->tcon_ipc->need_reconnect = true; diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index 4e4cc0cd5206..0bb2ac929061 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -296,6 +296,60 @@ int cifs_try_adding_channels(struct cifs_ses *ses) return new_chan_count - old_chan_count; } +/* + * called when multichannel is disabled by the server. + * this always gets called from smb2_reconnect + * and cannot get called in parallel threads. + */ +void +cifs_disable_secondary_channels(struct cifs_ses *ses) +{ + int i, chan_count; + struct TCP_Server_Info *server; + struct cifs_server_iface *iface; + + spin_lock(&ses->chan_lock); + chan_count = ses->chan_count; + if (chan_count == 1) + goto done; + + ses->chan_count = 1; + + /* for all secondary channels reset the need reconnect bit */ + ses->chans_need_reconnect &= 1; + + for (i = 1; i < chan_count; i++) { + iface = ses->chans[i].iface; + server = ses->chans[i].server; + + if (iface) { + spin_lock(&ses->iface_lock); + kref_put(&iface->refcount, release_iface); + ses->chans[i].iface = NULL; + iface->num_channels--; + if (iface->weight_fulfilled) + iface->weight_fulfilled--; + spin_unlock(&ses->iface_lock); + } + + spin_unlock(&ses->chan_lock); + if (server && !server->terminate) { + server->terminate = true; + cifs_signal_cifsd_for_reconnect(server, false); + } + spin_lock(&ses->chan_lock); + + if (server) { + ses->chans[i].server = NULL; + cifs_put_tcp_session(server, false); + } + + } + +done: + spin_unlock(&ses->chan_lock); +} + /* * update the iface for the channel if necessary. * will return 0 when iface is updated, 1 if removed, 2 otherwise @@ -595,14 +649,10 @@ cifs_ses_add_channel(struct cifs_ses *ses, out: if (rc && chan->server) { - /* - * we should avoid race with these delayed works before we - * remove this channel - */ - cancel_delayed_work_sync(&chan->server->echo); - cancel_delayed_work_sync(&chan->server->reconnect); + cifs_put_tcp_session(chan->server, 0); spin_lock(&ses->chan_lock); + /* we rely on all bits beyond chan_count to be clear */ cifs_chan_clear_need_reconnect(ses, chan->server); ses->chan_count--; @@ -612,8 +662,6 @@ out: */ WARN_ON(ses->chan_count < 1); spin_unlock(&ses->chan_lock); - - cifs_put_tcp_session(chan->server, 0); } kfree(ctx->UNC); diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 0d4351e43069..2eb29fa278c3 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -164,6 +164,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, struct nls_table *nls_codepage = NULL; struct cifs_ses *ses; int xid; + struct TCP_Server_Info *pserver; + unsigned int chan_index; /* * SMB2s NegProt, SessSetup, Logoff do not have tcon yet so @@ -224,6 +226,12 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, return -EAGAIN; } } + + /* if server is marked for termination, cifsd will cleanup */ + if (server->terminate) { + spin_unlock(&server->srv_lock); + return -EHOSTDOWN; + } spin_unlock(&server->srv_lock); again: @@ -242,12 +250,24 @@ again: tcon->need_reconnect); mutex_lock(&ses->session_mutex); + /* + * if this is called by delayed work, and the channel has been disabled + * in parallel, the delayed work can continue to execute in parallel + * there's a chance that this channel may not exist anymore + */ + spin_lock(&server->srv_lock); + if (server->tcpStatus == CifsExiting) { + spin_unlock(&server->srv_lock); + mutex_unlock(&ses->session_mutex); + rc = -EHOSTDOWN; + goto out; + } + /* * Recheck after acquire mutex. If another thread is negotiating * and the server never sends an answer the socket will be closed * and tcpStatus set to reconnect. */ - spin_lock(&server->srv_lock); if (server->tcpStatus == CifsNeedReconnect) { spin_unlock(&server->srv_lock); mutex_unlock(&ses->session_mutex); @@ -284,6 +304,53 @@ again: rc = cifs_negotiate_protocol(0, ses, server); if (!rc) { + /* + * if server stopped supporting multichannel + * and the first channel reconnected, disable all the others. + */ + if (ses->chan_count > 1 && + !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) { + if (SERVER_IS_CHAN(server)) { + cifs_dbg(VFS, "server %s does not support " \ + "multichannel anymore. skipping secondary channel\n", + ses->server->hostname); + + spin_lock(&ses->chan_lock); + chan_index = cifs_ses_get_chan_index(ses, server); + if (chan_index == CIFS_INVAL_CHAN_INDEX) { + spin_unlock(&ses->chan_lock); + goto skip_terminate; + } + + ses->chans[chan_index].server = NULL; + spin_unlock(&ses->chan_lock); + + /* + * the above reference of server by channel + * needs to be dropped without holding chan_lock + * as cifs_put_tcp_session takes a higher lock + * i.e. cifs_tcp_ses_lock + */ + cifs_put_tcp_session(server, 1); + + server->terminate = true; + cifs_signal_cifsd_for_reconnect(server, false); + + /* mark primary server as needing reconnect */ + pserver = server->primary_server; + cifs_signal_cifsd_for_reconnect(pserver, false); + +skip_terminate: + mutex_unlock(&ses->session_mutex); + rc = -EHOSTDOWN; + goto out; + } else { + cifs_server_dbg(VFS, "does not support " \ + "multichannel anymore. disabling all other channels\n"); + cifs_disable_secondary_channels(ses); + } + } + rc = cifs_setup_session(0, ses, server, nls_codepage); if ((rc == -EACCES) && !tcon->retry) { mutex_unlock(&ses->session_mutex); @@ -3836,6 +3903,13 @@ void smb2_reconnect_server(struct work_struct *work) /* Prevent simultaneous reconnects that can corrupt tcon->rlist list */ mutex_lock(&pserver->reconnect_mutex); + /* if the server is marked for termination, drop the ref count here */ + if (server->terminate) { + cifs_put_tcp_session(server, true); + mutex_unlock(&pserver->reconnect_mutex); + return; + } + INIT_LIST_HEAD(&tmp_list); INIT_LIST_HEAD(&tmp_ses_list); cifs_dbg(FYI, "Reconnecting tcons and channels\n"); diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c index d553b7a54621..4f717ad7c21b 100644 --- a/fs/smb/client/transport.c +++ b/fs/smb/client/transport.c @@ -1023,7 +1023,7 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses) spin_lock(&ses->chan_lock); for (i = 0; i < ses->chan_count; i++) { server = ses->chans[i].server; - if (!server) + if (!server || server->terminate) continue; /* From fd2bd7c0539e28f267a84da8d68f9378511b50a7 Mon Sep 17 00:00:00 2001 From: Steve French Date: Thu, 20 Jul 2023 08:30:32 -0500 Subject: [PATCH 16/16] cifs: update internal module version number for cifs.ko From 2.45 to 2.46 Signed-off-by: Steve French --- fs/smb/client/cifsfs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h index 8ca3d7606bb4..3adea10aa9da 100644 --- a/fs/smb/client/cifsfs.h +++ b/fs/smb/client/cifsfs.h @@ -152,6 +152,6 @@ extern const struct export_operations cifs_export_ops; #endif /* CONFIG_CIFS_NFSD_EXPORT */ /* when changing internal version - update following two lines at same time */ -#define SMB3_PRODUCT_BUILD 45 -#define CIFS_VERSION "2.45" +#define SMB3_PRODUCT_BUILD 46 +#define CIFS_VERSION "2.46" #endif /* _CIFSFS_H */