From 1907e0fec10c2498729d27550c460528e49abe1e Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 23 Feb 2023 08:15:38 +0000 Subject: cifs: Add some missing xas_retry() calls The xas_for_each loops added into fs/cifs/file.c need to go round again if indicated by xas_retry(). Fixes: b8713c4dbfa3 ("cifs: Add some helper functions") Signed-off-by: David Howells Reviewed-by: Paulo Alcantara (SUSE) cc: Shyam Prasad N cc: Rohith Surabattula cc: Tom Talpey cc: Jeff Layton cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French --- fs/cifs/file.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index ebfcaae8c437..879524bf7528 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -52,6 +52,8 @@ static void cifs_undirty_folios(struct inode *inode, loff_t start, unsigned int end = (start + len - 1) / PAGE_SIZE; xas_for_each_marked(&xas, folio, end, PAGECACHE_TAG_DIRTY) { + if (xas_retry(&xas, folio)) + continue; xas_pause(&xas); rcu_read_unlock(); folio_lock(folio); @@ -81,6 +83,8 @@ void cifs_pages_written_back(struct inode *inode, loff_t start, unsigned int len end = (start + len - 1) / PAGE_SIZE; xas_for_each(&xas, folio, end) { + if (xas_retry(&xas, folio)) + continue; if (!folio_test_writeback(folio)) { WARN_ONCE(1, "bad %x @%llx page %lx %lx\n", len, start, folio_index(folio), end); @@ -112,6 +116,8 @@ void cifs_pages_write_failed(struct inode *inode, loff_t start, unsigned int len end = (start + len - 1) / PAGE_SIZE; xas_for_each(&xas, folio, end) { + if (xas_retry(&xas, folio)) + continue; if (!folio_test_writeback(folio)) { WARN_ONCE(1, "bad %x @%llx page %lx %lx\n", len, start, folio_index(folio), end); -- cgit v1.2.3 From 4bd3e4c7e4e552aa148edd3f2a2326ccc3011122 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 23 Feb 2023 08:15:39 +0000 Subject: cifs: Fix an uninitialised variable Fix an uninitialised variable introduced in cifs. Fixes: 3d78fe73fa12 ("cifs: Build the RDMA SGE list directly from an iterator") Signed-off-by: David Howells Reviewed-by: Paulo Alcantara (SUSE) cc: Steve French cc: Shyam Prasad N cc: Rohith Surabattula cc: Tom Talpey cc: Jeff Layton cc: linux-cifs@vger.kernel.org cc: linux-rdma@vger.kernel.org Signed-off-by: Steve French --- fs/cifs/smbdirect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index 55b6e319a61d..0362ebd4fa0f 100644 --- a/fs/cifs/smbdirect.c +++ b/fs/cifs/smbdirect.c @@ -837,7 +837,7 @@ static int smbd_post_send_iter(struct smbd_connection *info, int data_length; struct smbd_request *request; struct smbd_data_transfer *packet; - int new_credits; + int new_credits = 0; wait_credit: /* Wait for send credits. A SMBD packet needs one credit */ -- cgit v1.2.3 From a21be1f5df85831147d5a1cbb5f2a13ec8c6852b Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Tue, 27 Dec 2022 14:04:29 +0000 Subject: cifs: match even the scope id for ipv6 addresses match_address function matches the scope id for ipv6 addresses, but cifs_match_ipaddr (which is another function used for comparison) does not use scope id. Doing so with this change. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/cifs/connect.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index ec020d860be3..5dabd8dc3e8e 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1294,7 +1294,8 @@ cifs_match_ipaddr(struct sockaddr *srcaddr, struct sockaddr *rhs) case AF_INET6: { struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr; struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)rhs; - return ipv6_addr_equal(&saddr6->sin6_addr, &vaddr6->sin6_addr); + return (ipv6_addr_equal(&saddr6->sin6_addr, &vaddr6->sin6_addr) + && saddr6->sin6_scope_id == vaddr6->sin6_scope_id); } default: WARN_ON(1); -- cgit v1.2.3 From 410612b0726b2ee68808da7bd27103d96b4cf898 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Tue, 27 Dec 2022 14:09:32 +0000 Subject: cifs: reuse cifs_match_ipaddr for comparison of dstaddr too We have two pieces of code that does pretty much the same comparison. This change reuses cifs_match_ipaddr within match_address. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/cifs/connect.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 5dabd8dc3e8e..5233f14f0636 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1344,32 +1344,8 @@ match_port(struct TCP_Server_Info *server, struct sockaddr *addr) static bool match_server_address(struct TCP_Server_Info *server, struct sockaddr *addr) { - switch (addr->sa_family) { - case AF_INET: { - struct sockaddr_in *addr4 = (struct sockaddr_in *)addr; - struct sockaddr_in *srv_addr4 = - (struct sockaddr_in *)&server->dstaddr; - - if (addr4->sin_addr.s_addr != srv_addr4->sin_addr.s_addr) - return false; - break; - } - case AF_INET6: { - struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr; - struct sockaddr_in6 *srv_addr6 = - (struct sockaddr_in6 *)&server->dstaddr; - - if (!ipv6_addr_equal(&addr6->sin6_addr, - &srv_addr6->sin6_addr)) - return false; - if (addr6->sin6_scope_id != srv_addr6->sin6_scope_id) - return false; - break; - } - default: - WARN_ON(1); - return false; /* don't expect to be here */ - } + if (!cifs_match_ipaddr(addr, (struct sockaddr *)&server->dstaddr)) + return false; return true; } -- cgit v1.2.3 From 0268792f77d2e4ba8e056173bdf2f9af9963be76 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 27 Feb 2023 13:04:53 +0000 Subject: cifs: Fix cifs_write_back_from_locked_folio() cifs_write_back_from_locked_folio() should return the number of bytes read, but returns the result of ->async_writev(), which will be 0 on success. As it happens, this doesn't prevent cifs_writepages_region() from working as it will then examine and ignore the pages that are no longer dirty rather than just skipping over them. Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list") Signed-off-by: David Howells cc: Shyam Prasad N cc: Rohith Surabattula cc: Tom Talpey cc: Jeff Layton cc: linux-cifs@vger.kernel.org Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/cifs/file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 879524bf7528..ec0694a65c7b 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -2845,6 +2845,7 @@ err_xid: free_xid(xid); if (rc == 0) { wbc->nr_to_write = count; + rc = len; } else if (is_retryable_error(rc)) { cifs_pages_write_redirty(inode, start, len); } else { -- cgit v1.2.3 From 4c0421fa6df136ff869a078594b4b7b7637e566a Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 27 Feb 2023 13:04:54 +0000 Subject: iov: Fix netfs_extract_user_to_sg() Fix the loop check in netfs_extract_user_to_sg() for extraction from user-backed iterators to do the body if npages > 0, not if npages < 0 (which it can never be). This isn't currently used by cifs, which only ever extracts data from BVEC, KVEC and XARRAY iterators at this level, user-backed iterators having being decanted into BVEC iterators at a higher level to accommodate the work being done in a kernel thread. Found by smatch: fs/netfs/iterator.c:139 netfs_extract_user_to_sg() warn: unsigned 'npages' is never less than zero. Fixes: 018584697533 ("netfs: Add a function to extract an iterator into a scatterlist") Reported-by: kernel test robot Link: https://lore.kernel.org/oe-kbuild-all/202302261115.P3TQi1ZO-lkp@intel.com/ Reported-by: Dan Carpenter Link: https://lore.kernel.org/r/Y/yYnAhoAYDBKixX@kili Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: David Howells cc: Jeff Layton cc: linux-cifs@vger.kernel.org cc: linux-cachefs@redhat.com Signed-off-by: Steve French --- fs/netfs/iterator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/netfs/iterator.c b/fs/netfs/iterator.c index f00d43b8ac0a..e9a45dea748a 100644 --- a/fs/netfs/iterator.c +++ b/fs/netfs/iterator.c @@ -134,7 +134,7 @@ static ssize_t netfs_extract_user_to_sg(struct iov_iter *iter, npages = DIV_ROUND_UP(off + len, PAGE_SIZE); sg_max -= npages; - for (; npages < 0; npages--) { + for (; npages > 0; npages--) { struct page *page = *pages; size_t seg = min_t(size_t, PAGE_SIZE - off, len); -- cgit v1.2.3 From b9ee2e307c6b06384b6f9e393a9b8e048e8fc277 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Tue, 28 Feb 2023 19:01:54 -0300 Subject: cifs: improve checking of DFS links over STATUS_OBJECT_NAME_INVALID Do not map STATUS_OBJECT_NAME_INVALID to -EREMOTE under non-DFS shares, or 'nodfs' mounts or CONFIG_CIFS_DFS_UPCALL=n builds. Otherwise, in the slow path, get a referral to figure out whether it is an actual DFS link. This could be simply reproduced under a non-DFS share by running the following $ mount.cifs //srv/share /mnt -o ... $ cat /mnt/$(printf '\U110000') cat: '/mnt/'$'\364\220\200\200': Object is remote Fixes: c877ce47e137 ("cifs: reduce roundtrips on create/qinfo requests") CC: stable@vger.kernel.org # 6.2 Signed-off-by: Paulo Alcantara (SUSE) Reviewed-by: Ronnie Sahlberg Signed-off-by: Steve French --- fs/cifs/cifsproto.h | 20 ++++++++++++---- fs/cifs/misc.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/cifs/smb2inode.c | 21 +++++++++-------- fs/cifs/smb2ops.c | 23 ++++++++++-------- 4 files changed, 106 insertions(+), 25 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index b7a36ebd0f2f..20a2f0f3f682 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -667,11 +667,21 @@ static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses, int match_target_ip(struct TCP_Server_Info *server, const char *share, size_t share_len, bool *result); - -int cifs_dfs_query_info_nonascii_quirk(const unsigned int xid, - struct cifs_tcon *tcon, - struct cifs_sb_info *cifs_sb, - const char *dfs_link_path); +int cifs_inval_name_dfs_link_error(const unsigned int xid, + struct cifs_tcon *tcon, + struct cifs_sb_info *cifs_sb, + const char *full_path, + bool *islink); +#else +static inline int cifs_inval_name_dfs_link_error(const unsigned int xid, + struct cifs_tcon *tcon, + struct cifs_sb_info *cifs_sb, + const char *full_path, + bool *islink) +{ + *islink = false; + return 0; +} #endif static inline int cifs_create_options(struct cifs_sb_info *cifs_sb, int options) diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 2905734eb289..0c6c1fc8dae9 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -21,6 +21,7 @@ #include "cifsfs.h" #ifdef CONFIG_CIFS_DFS_UPCALL #include "dns_resolve.h" +#include "dfs_cache.h" #endif #include "fs_context.h" #include "cached_dir.h" @@ -1198,4 +1199,70 @@ int cifs_update_super_prepath(struct cifs_sb_info *cifs_sb, char *prefix) cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_USE_PREFIX_PATH; return 0; } + +/* + * Handle weird Windows SMB server behaviour. It responds with + * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request for + * "\\\" DFS reference, where contains + * non-ASCII unicode symbols. + */ +int cifs_inval_name_dfs_link_error(const unsigned int xid, + struct cifs_tcon *tcon, + struct cifs_sb_info *cifs_sb, + const char *full_path, + bool *islink) +{ + struct cifs_ses *ses = tcon->ses; + size_t len; + char *path; + char *ref_path; + + *islink = false; + + /* + * Fast path - skip check when @full_path doesn't have a prefix path to + * look up or tcon is not DFS. + */ + if (strlen(full_path) < 2 || !cifs_sb || + (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS) || + !is_tcon_dfs(tcon) || !ses->server->origin_fullpath) + return 0; + + /* + * Slow path - tcon is DFS and @full_path has prefix path, so attempt + * to get a referral to figure out whether it is an DFS link. + */ + len = strnlen(tcon->tree_name, MAX_TREE_SIZE + 1) + strlen(full_path) + 1; + path = kmalloc(len, GFP_KERNEL); + if (!path) + return -ENOMEM; + + scnprintf(path, len, "%s%s", tcon->tree_name, full_path); + ref_path = dfs_cache_canonical_path(path + 1, cifs_sb->local_nls, + cifs_remap(cifs_sb)); + kfree(path); + + if (IS_ERR(ref_path)) { + if (PTR_ERR(ref_path) != -EINVAL) + return PTR_ERR(ref_path); + } else { + struct dfs_info3_param *refs = NULL; + int num_refs = 0; + + /* + * XXX: we are not using dfs_cache_find() here because we might + * end filling all the DFS cache and thus potentially + * removing cached DFS targets that the client would eventually + * need during failover. + */ + if (ses->server->ops->get_dfs_refer && + !ses->server->ops->get_dfs_refer(xid, ses, ref_path, &refs, + &num_refs, cifs_sb->local_nls, + cifs_remap(cifs_sb))) + *islink = refs[0].server_type == DFS_TYPE_LINK; + free_dfs_info_array(refs, num_refs); + kfree(ref_path); + } + return 0; +} #endif diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c index 37b4cd59245d..9b956294e864 100644 --- a/fs/cifs/smb2inode.c +++ b/fs/cifs/smb2inode.c @@ -527,12 +527,13 @@ int smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, struct cifs_sb_info *cifs_sb, const char *full_path, struct cifs_open_info_data *data, bool *adjust_tz, bool *reparse) { - int rc; __u32 create_options = 0; struct cifsFileInfo *cfile; struct cached_fid *cfid = NULL; struct kvec err_iov[3] = {}; int err_buftype[3] = {}; + bool islink; + int rc, rc2; *adjust_tz = false; *reparse = false; @@ -580,15 +581,15 @@ int smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, SMB2_OP_QUERY_INFO, cfile, NULL, NULL, NULL, NULL); goto out; - } else if (rc != -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && - hdr->Status == STATUS_OBJECT_NAME_INVALID) { - /* - * Handle weird Windows SMB server behaviour. It responds with - * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request - * for "\\\" DFS reference, - * where contains non-ASCII unicode symbols. - */ - rc = -EREMOTE; + } else if (rc != -EREMOTE && hdr->Status == STATUS_OBJECT_NAME_INVALID) { + rc2 = cifs_inval_name_dfs_link_error(xid, tcon, cifs_sb, + full_path, &islink); + if (rc2) { + rc = rc2; + goto out; + } + if (islink) + rc = -EREMOTE; } if (rc == -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && cifs_sb && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS)) diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index f79b075f2992..6dfb865ee9d7 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -796,7 +796,6 @@ static int smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon, struct cifs_sb_info *cifs_sb, const char *full_path) { - int rc; __le16 *utf16_path; __u8 oplock = SMB2_OPLOCK_LEVEL_NONE; int err_buftype = CIFS_NO_BUFFER; @@ -804,6 +803,8 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon, struct kvec err_iov = {}; struct cifs_fid fid; struct cached_fid *cfid; + bool islink; + int rc, rc2; rc = open_cached_dir(xid, tcon, full_path, cifs_sb, true, &cfid); if (!rc) { @@ -833,15 +834,17 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon, if (unlikely(!hdr || err_buftype == CIFS_NO_BUFFER)) goto out; - /* - * Handle weird Windows SMB server behaviour. It responds with - * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request - * for "\\\" DFS reference, - * where contains non-ASCII unicode symbols. - */ - if (rc != -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && - hdr->Status == STATUS_OBJECT_NAME_INVALID) - rc = -EREMOTE; + + if (rc != -EREMOTE && hdr->Status == STATUS_OBJECT_NAME_INVALID) { + rc2 = cifs_inval_name_dfs_link_error(xid, tcon, cifs_sb, + full_path, &islink); + if (rc2) { + rc = rc2; + goto out; + } + if (islink) + rc = -EREMOTE; + } if (rc == -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && cifs_sb && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS)) rc = -EOPNOTSUPP; -- cgit v1.2.3 From 1bcd548d935a33c6fc58331405eb1b82fd6150de Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Tue, 28 Feb 2023 19:01:55 -0300 Subject: cifs: prevent data race in cifs_reconnect_tcon() Make sure to get an up-to-date TCP_Server_Info::nr_targets value prior to waiting the server to be reconnected in cifs_reconnect_tcon(). It is set in cifs_tcp_ses_needs_reconnect() and protected by TCP_Server_Info::srv_lock. Create a new cifs_wait_for_server_reconnect() helper that can be used by both SMB2+ and CIFS reconnect code. Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/cifs/cifsproto.h | 1 + fs/cifs/cifssmb.c | 43 ++-------------------------- fs/cifs/misc.c | 44 ++++++++++++++++++++++++++++ fs/cifs/smb2pdu.c | 82 ++++++++++++++--------------------------------------- 4 files changed, 69 insertions(+), 101 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 20a2f0f3f682..e2eff66eefab 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -694,5 +694,6 @@ static inline int cifs_create_options(struct cifs_sb_info *cifs_sb, int options) struct super_block *cifs_get_tcon_super(struct cifs_tcon *tcon); void cifs_put_tcon_super(struct super_block *sb); +int cifs_wait_for_server_reconnect(struct TCP_Server_Info *server, bool retry); #endif /* _CIFSPROTO_H */ diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index a24e4ddf8043..a43c78396dd8 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -72,7 +72,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) struct cifs_ses *ses; struct TCP_Server_Info *server; struct nls_table *nls_codepage; - int retries; /* * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for @@ -102,45 +101,9 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) } spin_unlock(&tcon->tc_lock); - retries = server->nr_targets; - - /* - * Give demultiplex thread up to 10 seconds to each target available for - * reconnect -- should be greater than cifs socket timeout which is 7 - * seconds. - */ - while (server->tcpStatus == CifsNeedReconnect) { - rc = wait_event_interruptible_timeout(server->response_q, - (server->tcpStatus != CifsNeedReconnect), - 10 * HZ); - if (rc < 0) { - cifs_dbg(FYI, "%s: aborting reconnect due to a received signal by the process\n", - __func__); - return -ERESTARTSYS; - } - - /* are we still trying to reconnect? */ - spin_lock(&server->srv_lock); - if (server->tcpStatus != CifsNeedReconnect) { - spin_unlock(&server->srv_lock); - break; - } - spin_unlock(&server->srv_lock); - - if (retries && --retries) - continue; - - /* - * on "soft" mounts we wait once. Hard mounts keep - * retrying until process is killed or server comes - * back on-line - */ - if (!tcon->retry) { - cifs_dbg(FYI, "gave up waiting on reconnect in smb_init\n"); - return -EHOSTDOWN; - } - retries = server->nr_targets; - } + rc = cifs_wait_for_server_reconnect(server, tcon->retry); + if (rc) + return rc; spin_lock(&ses->chan_lock); if (!cifs_chan_needs_reconnect(ses, server) && !tcon->need_reconnect) { diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 0c6c1fc8dae9..a0d286ee723d 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -1266,3 +1266,47 @@ int cifs_inval_name_dfs_link_error(const unsigned int xid, return 0; } #endif + +int cifs_wait_for_server_reconnect(struct TCP_Server_Info *server, bool retry) +{ + int timeout = 10; + int rc; + + spin_lock(&server->srv_lock); + if (server->tcpStatus != CifsNeedReconnect) { + spin_unlock(&server->srv_lock); + return 0; + } + timeout *= server->nr_targets; + spin_unlock(&server->srv_lock); + + /* + * Give demultiplex thread up to 10 seconds to each target available for + * reconnect -- should be greater than cifs socket timeout which is 7 + * seconds. + * + * On "soft" mounts we wait once. Hard mounts keep retrying until + * process is killed or server comes back on-line. + */ + do { + rc = wait_event_interruptible_timeout(server->response_q, + (server->tcpStatus != CifsNeedReconnect), + timeout * HZ); + if (rc < 0) { + cifs_dbg(FYI, "%s: aborting reconnect due to received signal\n", + __func__); + return -ERESTARTSYS; + } + + /* are we still trying to reconnect? */ + spin_lock(&server->srv_lock); + if (server->tcpStatus != CifsNeedReconnect) { + spin_unlock(&server->srv_lock); + return 0; + } + spin_unlock(&server->srv_lock); + } while (retry); + + cifs_dbg(FYI, "%s: gave up waiting on reconnect\n", __func__); + return -EHOSTDOWN; +} diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index ca9d7110ddcb..0e53265e1462 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -139,66 +139,6 @@ out: return; } -static int wait_for_server_reconnect(struct TCP_Server_Info *server, - __le16 smb2_command, bool retry) -{ - int timeout = 10; - int rc; - - spin_lock(&server->srv_lock); - if (server->tcpStatus != CifsNeedReconnect) { - spin_unlock(&server->srv_lock); - return 0; - } - timeout *= server->nr_targets; - spin_unlock(&server->srv_lock); - - /* - * Return to caller for TREE_DISCONNECT and LOGOFF and CLOSE - * here since they are implicitly done when session drops. - */ - switch (smb2_command) { - /* - * BB Should we keep oplock break and add flush to exceptions? - */ - case SMB2_TREE_DISCONNECT: - case SMB2_CANCEL: - case SMB2_CLOSE: - case SMB2_OPLOCK_BREAK: - return -EAGAIN; - } - - /* - * Give demultiplex thread up to 10 seconds to each target available for - * reconnect -- should be greater than cifs socket timeout which is 7 - * seconds. - * - * On "soft" mounts we wait once. Hard mounts keep retrying until - * process is killed or server comes back on-line. - */ - do { - rc = wait_event_interruptible_timeout(server->response_q, - (server->tcpStatus != CifsNeedReconnect), - timeout * HZ); - if (rc < 0) { - cifs_dbg(FYI, "%s: aborting reconnect due to received signal\n", - __func__); - return -ERESTARTSYS; - } - - /* are we still trying to reconnect? */ - spin_lock(&server->srv_lock); - if (server->tcpStatus != CifsNeedReconnect) { - spin_unlock(&server->srv_lock); - return 0; - } - spin_unlock(&server->srv_lock); - } while (retry); - - cifs_dbg(FYI, "%s: gave up waiting on reconnect\n", __func__); - return -EHOSTDOWN; -} - static int smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, struct TCP_Server_Info *server) @@ -243,7 +183,27 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, (!tcon->ses->server) || !server) return -EIO; - rc = wait_for_server_reconnect(server, smb2_command, tcon->retry); + spin_lock(&server->srv_lock); + if (server->tcpStatus == CifsNeedReconnect) { + /* + * Return to caller for TREE_DISCONNECT and LOGOFF and CLOSE + * here since they are implicitly done when session drops. + */ + switch (smb2_command) { + /* + * BB Should we keep oplock break and add flush to exceptions? + */ + case SMB2_TREE_DISCONNECT: + case SMB2_CANCEL: + case SMB2_CLOSE: + case SMB2_OPLOCK_BREAK: + spin_unlock(&server->srv_lock); + return -EAGAIN; + } + } + spin_unlock(&server->srv_lock); + + rc = cifs_wait_for_server_reconnect(server, tcon->retry); if (rc) return rc; -- cgit v1.2.3 From 71562809e401b2f5ad371d99ce0323e988406fd6 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 28 Feb 2023 22:38:38 +0000 Subject: cifs: Fix memory leak in direct I/O When __cifs_readv() and __cifs_writev() extract pages from a user-backed iterator into a BVEC-type iterator, they set ->bv_need_unpin to note whether they need to unpin the pages later. However, in both cases they examine the BVEC-type iterator and not the source iterator - and so bv_need_unpin doesn't get set and the pages are leaked. I think this may be responsible for the generic/208 xfstest failing occasionally with: WARNING: CPU: 0 PID: 3064 at mm/gup.c:218 try_grab_page+0x65/0x100 RIP: 0010:try_grab_page+0x65/0x100 follow_page_pte+0x1a7/0x570 __get_user_pages+0x1a2/0x650 __gup_longterm_locked+0xdc/0xb50 internal_get_user_pages_fast+0x17f/0x310 pin_user_pages_fast+0x46/0x60 iov_iter_extract_pages+0xc9/0x510 ? __kmalloc_large_node+0xb1/0x120 ? __kmalloc_node+0xbe/0x130 netfs_extract_user_iter+0xbf/0x200 [netfs] __cifs_writev+0x150/0x330 [cifs] vfs_write+0x2a8/0x3c0 ksys_pwrite64+0x65/0xa0 with the page refcount going negative. This is less unlikely than it seems because the page is being pinned, not simply got, and so the refcount increased by 1024 each time, and so only needs to be called around ~2097152 for the refcount to go negative. Further, the test program (aio-dio-invalidate-failure) uses a 32MiB static buffer and all the PTEs covering it refer to the same page because it's never written to. The warning in try_grab_page(): if (WARN_ON_ONCE(folio_ref_count(folio) <= 0)) return -ENOMEM; then trips and prevents us ever using the page again for DIO at least. Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list") Reported-by: Murphy Zhou Link: https://lore.kernel.org/r/CAH2r5mvaTsJ---n=265a4zqRA7pP+o4MJ36WCQUS6oPrOij8cw@mail.gmail.com Signed-off-by: David Howells Reviewed-by: Paulo Alcantara (SUSE) cc: Shyam Prasad N cc: Rohith Surabattula cc: Jeff Layton cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French --- fs/cifs/file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index ec0694a65c7b..4d4a2d82636d 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -3612,7 +3612,7 @@ static ssize_t __cifs_writev( ctx->nr_pinned_pages = rc; ctx->bv = (void *)ctx->iter.bvec; - ctx->bv_need_unpin = iov_iter_extract_will_pin(&ctx->iter); + ctx->bv_need_unpin = iov_iter_extract_will_pin(from); } else if ((iov_iter_is_bvec(from) || iov_iter_is_kvec(from)) && !is_sync_kiocb(iocb)) { /* @@ -4148,7 +4148,7 @@ static ssize_t __cifs_readv( ctx->nr_pinned_pages = rc; ctx->bv = (void *)ctx->iter.bvec; - ctx->bv_need_unpin = iov_iter_extract_will_pin(&ctx->iter); + ctx->bv_need_unpin = iov_iter_extract_will_pin(to); ctx->should_dirty = true; } else if ((iov_iter_is_bvec(to) || iov_iter_is_kvec(to)) && !is_sync_kiocb(iocb)) { -- cgit v1.2.3