From da787d5b74983f7525d1eb4b9c0b4aff2821511a Mon Sep 17 00:00:00 2001 From: Bharath SM Date: Sun, 18 Jun 2023 19:02:24 +0000 Subject: SMB3: Do not send lease break acknowledgment if all file handles have been closed In case if all existing file handles are deferred handles and if all of them gets closed due to handle lease break then we dont need to send lease break acknowledgment to server, because last handle close will be considered as lease break ack. After closing deferred handels, we check for openfile list of inode, if its empty then we skip sending lease break ack. Fixes: 59a556aebc43 ("SMB3: drop reference to cfile before sending oplock break") Reviewed-by: Tom Talpey Signed-off-by: Bharath SM Signed-off-by: Steve French --- fs/smb/client/file.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 051283386e22..1a854dc20482 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -4936,20 +4936,19 @@ oplock_break_ack: _cifsFileInfo_put(cfile, false /* do not wait for ourself */, false); /* - * releasing stale oplock after recent reconnect of smb session using - * a now incorrect file handle is not a data integrity issue but do - * not bother sending an oplock release if session to server still is - * disconnected since oplock already released by the server + * MS-SMB2 3.2.5.19.1 and 3.2.5.19.2 (and MS-CIFS 3.2.5.42) do not require + * an acknowledgment to be sent when the file has already been closed. + * check for server null, since can race with kill_sb calling tree disconnect. */ - if (!oplock_break_cancelled) { - /* check for server null since can race with kill_sb calling tree disconnect */ - if (tcon->ses && tcon->ses->server) { - rc = tcon->ses->server->ops->oplock_response(tcon, persistent_fid, - volatile_fid, net_fid, cinode); - cifs_dbg(FYI, "Oplock release rc = %d\n", rc); - } else - pr_warn_once("lease break not sent for unmounted share\n"); - } + spin_lock(&cinode->open_file_lock); + if (tcon->ses && tcon->ses->server && !oplock_break_cancelled && + !list_empty(&cinode->openFileList)) { + spin_unlock(&cinode->open_file_lock); + rc = tcon->ses->server->ops->oplock_response(tcon, persistent_fid, + volatile_fid, net_fid, cinode); + cifs_dbg(FYI, "Oplock release rc = %d\n", rc); + } else + spin_unlock(&cinode->open_file_lock); cifs_done_oplock_break(cinode); } -- cgit v1.2.3 From dc765027ed2941985fbb8ef86139e6289b36fc43 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Fri, 16 Jun 2023 10:37:45 +0000 Subject: cifs: print nosharesock value while dumping mount options We print most other mount options for a mount when dumping the mount entries. But miss out the nosharesock value. This change will print that in addition to the other options. Signed-off-by: Shyam Prasad N Reviewed-by: Bharath SM Signed-off-by: Steve French --- fs/smb/client/cifsfs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/smb/client') diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index 43a4d8603db3..86ac620a9615 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -688,6 +688,8 @@ cifs_show_options(struct seq_file *s, struct dentry *root) seq_puts(s, ",noautotune"); if (tcon->ses->server->noblocksnd) seq_puts(s, ",noblocksend"); + if (tcon->ses->server->nosharesock) + seq_puts(s, ",nosharesock"); if (tcon->snapshot_time) seq_printf(s, ",snapshot=%llu", tcon->snapshot_time); -- cgit v1.2.3 From fc1bd51d110e206da5bee07e889d285c267a6874 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Mon, 19 Jun 2023 16:52:01 -0300 Subject: smb: client: fix warning in cifs_match_super() Fix potential dereference of ERR_PTR @tlink as reported by kernel test robot fs/smb/client/connect.c:2775 cifs_match_super() error: 'tlink' dereferencing possible ERR_PTR() Link: https://lore.kernel.org/all/202306170124.CtQqzf0I-lkp@intel.com/ Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/connect.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 9d16626e7a66..f9e0b59802d5 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -2767,8 +2767,9 @@ cifs_match_super(struct super_block *sb, void *data) } tlink = cifs_get_tlink(cifs_sb_master_tlink(cifs_sb)); - if (tlink == NULL) { - /* can not match superblock if tlink were ever null */ + if (IS_ERR_OR_NULL(tlink)) { + pr_warn_once("%s: skip super matching due to bad tlink(%p)\n", + __func__, tlink); spin_unlock(&cifs_tcp_ses_lock); return 0; } -- cgit v1.2.3 From 12c30f33cc6769bf411088a2872843c4f9ea32f9 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Mon, 19 Jun 2023 16:24:37 -0300 Subject: smb: client: fix warning in cifs_smb3_do_mount() This fixes the following warning reported by kernel test robot fs/smb/client/cifsfs.c:982 cifs_smb3_do_mount() warn: possible memory leak of 'cifs_sb' Link: https://lore.kernel.org/all/202306170124.CtQqzf0I-lkp@intel.com/ Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/cifsfs.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index 86ac620a9615..d499e18fefea 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -886,11 +886,11 @@ struct dentry * cifs_smb3_do_mount(struct file_system_type *fs_type, int flags, struct smb3_fs_context *old_ctx) { - int rc; - struct super_block *sb = NULL; - struct cifs_sb_info *cifs_sb = NULL; struct cifs_mnt_data mnt_data; + struct cifs_sb_info *cifs_sb; + struct super_block *sb; struct dentry *root; + int rc; if (cifsFYI) { cifs_dbg(FYI, "%s: devname=%s flags=0x%x\n", __func__, @@ -899,11 +899,9 @@ cifs_smb3_do_mount(struct file_system_type *fs_type, cifs_info("Attempting to mount %s\n", old_ctx->source); } - cifs_sb = kzalloc(sizeof(struct cifs_sb_info), GFP_KERNEL); - if (cifs_sb == NULL) { - root = ERR_PTR(-ENOMEM); - goto out; - } + cifs_sb = kzalloc(sizeof(*cifs_sb), GFP_KERNEL); + if (!cifs_sb) + return ERR_PTR(-ENOMEM); cifs_sb->ctx = kzalloc(sizeof(struct smb3_fs_context), GFP_KERNEL); if (!cifs_sb->ctx) { @@ -940,10 +938,8 @@ cifs_smb3_do_mount(struct file_system_type *fs_type, sb = sget(fs_type, cifs_match_super, cifs_set_super, flags, &mnt_data); if (IS_ERR(sb)) { - root = ERR_CAST(sb); cifs_umount(cifs_sb); - cifs_sb = NULL; - goto out; + return ERR_CAST(sb); } if (sb->s_root) { @@ -974,13 +970,9 @@ out_super: deactivate_locked_super(sb); return root; out: - if (cifs_sb) { - if (!sb || IS_ERR(sb)) { /* otherwise kill_sb will handle */ - kfree(cifs_sb->prepath); - smb3_cleanup_fs_context(cifs_sb->ctx); - kfree(cifs_sb); - } - } + kfree(cifs_sb->prepath); + smb3_cleanup_fs_context(cifs_sb->ctx); + kfree(cifs_sb); return root; } -- cgit v1.2.3 From acf35d79ee8c1cce0f879efe6446cf81e5491c36 Mon Sep 17 00:00:00 2001 From: Steve French Date: Mon, 19 Jun 2023 20:45:33 -0500 Subject: cifs: print more detail when invalidate_inode_mapping fails We had seen cases where cifs_invalidate_mapping was logging: "Could not invalidate inode ..." if invalidate_inode_pages2 fails but this message does not show what the rc is. Update the logged message to also log the return code. Suggested-by: Shyam Prasad N Reviewed-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index 1087ac6104a9..c3eeae07e139 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -2344,8 +2344,8 @@ cifs_invalidate_mapping(struct inode *inode) if (inode->i_mapping && inode->i_mapping->nrpages != 0) { rc = invalidate_inode_pages2(inode->i_mapping); if (rc) - cifs_dbg(VFS, "%s: Could not invalidate inode %p\n", - __func__, inode); + cifs_dbg(VFS, "%s: invalidate inode %p failed with rc %d\n", + __func__, inode, rc); } return rc; -- cgit v1.2.3 From e8eeca0bf4466ee1b196346d3a247535990cf44d Mon Sep 17 00:00:00 2001 From: Steve French Date: Mon, 19 Jun 2023 22:32:38 -0500 Subject: smb3: do not reserve too many oplock credits There were cases reported where servers will sometimes return more credits than requested on oplock break responses, which can lead to most of the credits being allocated for oplock breaks (instead of for normal operations like read and write) if number of SMB3 requests in flight always stays above 0 (the oplock and echo credits are rebalanced when in flight requests goes down to zero). If oplock credits gets unexpectedly large (e.g. three is more than it would ever be expected to be) and in flight requests are greater than zero, then rebalance the oplock credits and regular credits (go back to reserving just one oplock credit). Signed-off-by: Steve French --- fs/smb/client/smb2ops.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index a8bb9d00d33a..1dc2143ae924 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -109,7 +109,11 @@ smb2_add_credits(struct TCP_Server_Info *server, server->credits--; server->oplock_credits++; } - } + } else if ((server->in_flight > 0) && (server->oplock_credits > 3) && + ((optype & CIFS_OP_MASK) == CIFS_OBREAK_OP)) + /* if now have too many oplock credits, rebalance so don't starve normal ops */ + change_conf(server); + scredits = *val; in_flight = server->in_flight; spin_unlock(&server->req_lock); -- cgit v1.2.3 From 032137fe136a6073dcc699ee15fa3fd05fd77f21 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Mon, 19 Jun 2023 17:58:52 -0300 Subject: smb: client: fix warning in CIFSFindFirst() This fixes the following warning reported by kernel test robot fs/smb/client/cifssmb.c:4089 CIFSFindFirst() warn: missing error code? 'rc' Link: https://lore.kernel.org/all/202306170124.CtQqzf0I-lkp@intel.com/ Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/cifssmb.c | 98 ++++++++++++++++++++++--------------------------- 1 file changed, 44 insertions(+), 54 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c index 9d963caec35c..25d2509e520c 100644 --- a/fs/smb/client/cifssmb.c +++ b/fs/smb/client/cifssmb.c @@ -3958,11 +3958,12 @@ CIFSFindFirst(const unsigned int xid, struct cifs_tcon *tcon, TRANSACTION2_FFIRST_REQ *pSMB = NULL; TRANSACTION2_FFIRST_RSP *pSMBr = NULL; T2_FFIRST_RSP_PARMS *parms; - int rc = 0; + struct nls_table *nls_codepage; + unsigned int lnoff; + __u16 params, byte_count; int bytes_returned = 0; int name_len, remap; - __u16 params, byte_count; - struct nls_table *nls_codepage; + int rc = 0; cifs_dbg(FYI, "In FindFirst for %s\n", searchName); @@ -4043,63 +4044,52 @@ findFirstRetry: (struct smb_hdr *) pSMBr, &bytes_returned, 0); cifs_stats_inc(&tcon->stats.cifs_stats.num_ffirst); - if (rc) {/* BB add logic to retry regular search if Unix search - rejected unexpectedly by server */ - /* BB Add code to handle unsupported level rc */ + if (rc) { + /* + * BB: add logic to retry regular search if Unix search rejected + * unexpectedly by server. + */ + /* BB: add code to handle unsupported level rc */ cifs_dbg(FYI, "Error in FindFirst = %d\n", rc); - cifs_buf_release(pSMB); - - /* BB eventually could optimize out free and realloc of buf */ - /* for this case */ + /* + * BB: eventually could optimize out free and realloc of buf for + * this case. + */ if (rc == -EAGAIN) goto findFirstRetry; - } else { /* decode response */ - /* BB remember to free buffer if error BB */ - rc = validate_t2((struct smb_t2_rsp *)pSMBr); - if (rc == 0) { - unsigned int lnoff; - - if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE) - psrch_inf->unicode = true; - else - psrch_inf->unicode = false; - - psrch_inf->ntwrk_buf_start = (char *)pSMBr; - psrch_inf->smallBuf = false; - psrch_inf->srch_entries_start = - (char *) &pSMBr->hdr.Protocol + - le16_to_cpu(pSMBr->t2.DataOffset); - parms = (T2_FFIRST_RSP_PARMS *)((char *) &pSMBr->hdr.Protocol + - le16_to_cpu(pSMBr->t2.ParameterOffset)); - - if (parms->EndofSearch) - psrch_inf->endOfSearch = true; - else - psrch_inf->endOfSearch = false; - - psrch_inf->entries_in_buffer = - le16_to_cpu(parms->SearchCount); - psrch_inf->index_of_last_entry = 2 /* skip . and .. */ + - psrch_inf->entries_in_buffer; - lnoff = le16_to_cpu(parms->LastNameOffset); - if (CIFSMaxBufSize < lnoff) { - cifs_dbg(VFS, "ignoring corrupt resume name\n"); - psrch_inf->last_entry = NULL; - return rc; - } - - psrch_inf->last_entry = psrch_inf->srch_entries_start + - lnoff; - - if (pnetfid) - *pnetfid = parms->SearchHandle; - } else { - cifs_buf_release(pSMB); - } + return rc; + } + /* decode response */ + rc = validate_t2((struct smb_t2_rsp *)pSMBr); + if (rc) { + cifs_buf_release(pSMB); + return rc; } - return rc; + psrch_inf->unicode = !!(pSMBr->hdr.Flags2 & SMBFLG2_UNICODE); + psrch_inf->ntwrk_buf_start = (char *)pSMBr; + psrch_inf->smallBuf = false; + psrch_inf->srch_entries_start = (char *)&pSMBr->hdr.Protocol + + le16_to_cpu(pSMBr->t2.DataOffset); + + parms = (T2_FFIRST_RSP_PARMS *)((char *)&pSMBr->hdr.Protocol + + le16_to_cpu(pSMBr->t2.ParameterOffset)); + psrch_inf->endOfSearch = !!parms->EndofSearch; + + psrch_inf->entries_in_buffer = le16_to_cpu(parms->SearchCount); + psrch_inf->index_of_last_entry = 2 /* skip . and .. */ + + psrch_inf->entries_in_buffer; + lnoff = le16_to_cpu(parms->LastNameOffset); + if (CIFSMaxBufSize < lnoff) { + cifs_dbg(VFS, "ignoring corrupt resume name\n"); + psrch_inf->last_entry = NULL; + } else { + psrch_inf->last_entry = psrch_inf->srch_entries_start + lnoff; + if (pnetfid) + *pnetfid = parms->SearchHandle; + } + return 0; } int CIFSFindNext(const unsigned int xid, struct cifs_tcon *tcon, -- cgit v1.2.3 From 215533f888dcf18f7cfbbf520bdd52e67ac6265a Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Mon, 19 Jun 2023 18:41:00 -0300 Subject: smb: client: fix warning in CIFSFindNext() This fixes the following warning reported by kernel test robot fs/smb/client/cifssmb.c:4216 CIFSFindNext() warn: missing error code? 'rc' Link: https://lore.kernel.org/all/202306170124.CtQqzf0I-lkp@intel.com/ Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/cifssmb.c | 111 ++++++++++++++++++++++-------------------------- 1 file changed, 51 insertions(+), 60 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c index 25d2509e520c..19f7385abeec 100644 --- a/fs/smb/client/cifssmb.c +++ b/fs/smb/client/cifssmb.c @@ -4099,11 +4099,12 @@ int CIFSFindNext(const unsigned int xid, struct cifs_tcon *tcon, TRANSACTION2_FNEXT_REQ *pSMB = NULL; TRANSACTION2_FNEXT_RSP *pSMBr = NULL; T2_FNEXT_RSP_PARMS *parms; - char *response_data; - int rc = 0; - int bytes_returned; unsigned int name_len; + unsigned int lnoff; __u16 params, byte_count; + char *response_data; + int bytes_returned; + int rc = 0; cifs_dbg(FYI, "In FindNext\n"); @@ -4148,8 +4149,8 @@ int CIFSFindNext(const unsigned int xid, struct cifs_tcon *tcon, pSMB->ResumeFileName[name_len] = 0; pSMB->ResumeFileName[name_len+1] = 0; } else { - rc = -EINVAL; - goto FNext2_err_exit; + cifs_buf_release(pSMB); + return -EINVAL; } byte_count = params + 1 /* pad */ ; pSMB->TotalParameterCount = cpu_to_le16(params); @@ -4160,71 +4161,61 @@ int CIFSFindNext(const unsigned int xid, struct cifs_tcon *tcon, rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB, (struct smb_hdr *) pSMBr, &bytes_returned, 0); cifs_stats_inc(&tcon->stats.cifs_stats.num_fnext); + if (rc) { + cifs_buf_release(pSMB); if (rc == -EBADF) { psrch_inf->endOfSearch = true; - cifs_buf_release(pSMB); rc = 0; /* search probably was closed at end of search*/ - } else + } else { cifs_dbg(FYI, "FindNext returned = %d\n", rc); - } else { /* decode response */ - rc = validate_t2((struct smb_t2_rsp *)pSMBr); - - if (rc == 0) { - unsigned int lnoff; - - /* BB fixme add lock for file (srch_info) struct here */ - if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE) - psrch_inf->unicode = true; - else - psrch_inf->unicode = false; - response_data = (char *) &pSMBr->hdr.Protocol + - le16_to_cpu(pSMBr->t2.ParameterOffset); - parms = (T2_FNEXT_RSP_PARMS *)response_data; - response_data = (char *)&pSMBr->hdr.Protocol + - le16_to_cpu(pSMBr->t2.DataOffset); - if (psrch_inf->smallBuf) - cifs_small_buf_release( - psrch_inf->ntwrk_buf_start); - else - cifs_buf_release(psrch_inf->ntwrk_buf_start); - psrch_inf->srch_entries_start = response_data; - psrch_inf->ntwrk_buf_start = (char *)pSMB; - psrch_inf->smallBuf = false; - if (parms->EndofSearch) - psrch_inf->endOfSearch = true; - else - psrch_inf->endOfSearch = false; - psrch_inf->entries_in_buffer = - le16_to_cpu(parms->SearchCount); - psrch_inf->index_of_last_entry += - psrch_inf->entries_in_buffer; - lnoff = le16_to_cpu(parms->LastNameOffset); - if (CIFSMaxBufSize < lnoff) { - cifs_dbg(VFS, "ignoring corrupt resume name\n"); - psrch_inf->last_entry = NULL; - return rc; - } else - psrch_inf->last_entry = - psrch_inf->srch_entries_start + lnoff; - -/* cifs_dbg(FYI, "fnxt2 entries in buf %d index_of_last %d\n", - psrch_inf->entries_in_buffer, psrch_inf->index_of_last_entry); */ - - /* BB fixme add unlock here */ } + return rc; + } + /* decode response */ + rc = validate_t2((struct smb_t2_rsp *)pSMBr); + if (rc) { + cifs_buf_release(pSMB); + return rc; } + /* BB fixme add lock for file (srch_info) struct here */ + psrch_inf->unicode = !!(pSMBr->hdr.Flags2 & SMBFLG2_UNICODE); + response_data = (char *)&pSMBr->hdr.Protocol + + le16_to_cpu(pSMBr->t2.ParameterOffset); + parms = (T2_FNEXT_RSP_PARMS *)response_data; + response_data = (char *)&pSMBr->hdr.Protocol + + le16_to_cpu(pSMBr->t2.DataOffset); - /* BB On error, should we leave previous search buf (and count and - last entry fields) intact or free the previous one? */ + if (psrch_inf->smallBuf) + cifs_small_buf_release(psrch_inf->ntwrk_buf_start); + else + cifs_buf_release(psrch_inf->ntwrk_buf_start); - /* Note: On -EAGAIN error only caller can retry on handle based calls - since file handle passed in no longer valid */ -FNext2_err_exit: - if (rc != 0) - cifs_buf_release(pSMB); - return rc; + psrch_inf->srch_entries_start = response_data; + psrch_inf->ntwrk_buf_start = (char *)pSMB; + psrch_inf->smallBuf = false; + psrch_inf->endOfSearch = !!parms->EndofSearch; + psrch_inf->entries_in_buffer = le16_to_cpu(parms->SearchCount); + psrch_inf->index_of_last_entry += psrch_inf->entries_in_buffer; + lnoff = le16_to_cpu(parms->LastNameOffset); + if (CIFSMaxBufSize < lnoff) { + cifs_dbg(VFS, "ignoring corrupt resume name\n"); + psrch_inf->last_entry = NULL; + } else { + psrch_inf->last_entry = + psrch_inf->srch_entries_start + lnoff; + } + /* BB fixme add unlock here */ + + /* + * BB: On error, should we leave previous search buf + * (and count and last entry fields) intact or free the previous one? + * + * Note: On -EAGAIN error only caller can retry on handle based calls + * since file handle passed in no longer valid. + */ + return 0; } int -- cgit v1.2.3 From f0b6a834a8f0d267a112b150827bb65d4fdc471c Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Mon, 19 Jun 2023 19:23:13 -0300 Subject: smb: client: fix warning in generic_ip_connect() This fixes the following warning reported by kernel test robot fs/smb/client/connect.c:2974 generic_ip_connect() error: we previously assumed 'socket' could be null (see line 2962) Link: https://lore.kernel.org/all/202306170124.CtQqzf0I-lkp@intel.com/ Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/connect.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index f9e0b59802d5..972bc0804054 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -2934,11 +2934,11 @@ ip_rfc1001_connect(struct TCP_Server_Info *server) static int generic_ip_connect(struct TCP_Server_Info *server) { - int rc = 0; - __be16 sport; - int slen, sfamily; - struct socket *socket = server->ssocket; struct sockaddr *saddr; + struct socket *socket; + int slen, sfamily; + __be16 sport; + int rc = 0; saddr = (struct sockaddr *) &server->dstaddr; @@ -2960,18 +2960,19 @@ generic_ip_connect(struct TCP_Server_Info *server) ntohs(sport)); } - if (socket == NULL) { + if (server->ssocket) { + socket = server->ssocket; + } else { rc = __sock_create(cifs_net_ns(server), sfamily, SOCK_STREAM, - IPPROTO_TCP, &socket, 1); + IPPROTO_TCP, &server->ssocket, 1); if (rc < 0) { cifs_server_dbg(VFS, "Error %d creating socket\n", rc); - server->ssocket = NULL; return rc; } /* BB other socket options to set KEEPALIVE, NODELAY? */ cifs_dbg(FYI, "Socket created\n"); - server->ssocket = socket; + socket = server->ssocket; socket->sk->sk_allocation = GFP_NOFS; socket->sk->sk_use_task_frag = false; if (sfamily == AF_INET6) -- cgit v1.2.3 From 33f736187d08f6bc822117629f263b97d3df4165 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Thu, 22 Jun 2023 18:16:03 +0000 Subject: cifs: prevent use-after-free by freeing the cfile later In smb2_compound_op we have a possible use-after-free which can cause hard to debug problems later on. This was revealed during stress testing with KASAN enabled kernel. Fixing it by moving the cfile free call to a few lines below, after the usage. Fixes: 76894f3e2f71 ("cifs: improve symlink handling for smb2+") Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/smb2inode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index 163a03298430..7e3ac4cb4efa 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -398,9 +398,6 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, rsp_iov); finished: - if (cfile) - cifsFileInfo_put(cfile); - SMB2_open_free(&rqst[0]); if (rc == -EREMCHG) { pr_warn_once("server share %s deleted\n", tcon->tree_name); @@ -529,6 +526,9 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, break; } + if (cfile) + cifsFileInfo_put(cfile); + if (rc && err_iov && err_buftype) { memcpy(err_iov, rsp_iov, 3 * sizeof(*err_iov)); memcpy(err_buftype, resp_buftype, 3 * sizeof(*err_buftype)); -- cgit v1.2.3 From 326a8d04f147e2bf393f6f9cdb74126ee6900607 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Thu, 22 Jun 2023 18:16:04 +0000 Subject: cifs: do all necessary checks for credits within or before locking All the server credits and in-flight info is protected by req_lock. Once the req_lock is held, and we've determined that we have enough credits to continue, this lock cannot be dropped till we've made the changes to credits and in-flight count. However, we used to drop the lock in order to avoid deadlock with the recent srv_lock. This could cause the checks already made to be invalidated. Fixed it by moving the server status check to before locking req_lock. Fixes: d7d7a66aacd6 ("cifs: avoid use of global locks for high contention data") Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/smb2ops.c | 19 ++++++++++--------- fs/smb/client/transport.c | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 1dc2143ae924..3696d4ce0df3 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -215,6 +215,16 @@ smb2_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size, spin_lock(&server->req_lock); while (1) { + spin_unlock(&server->req_lock); + + spin_lock(&server->srv_lock); + if (server->tcpStatus == CifsExiting) { + spin_unlock(&server->srv_lock); + return -ENOENT; + } + spin_unlock(&server->srv_lock); + + spin_lock(&server->req_lock); if (server->credits <= 0) { spin_unlock(&server->req_lock); cifs_num_waiters_inc(server); @@ -225,15 +235,6 @@ smb2_wait_mtu_credits(struct TCP_Server_Info *server, unsigned int size, return rc; spin_lock(&server->req_lock); } else { - spin_unlock(&server->req_lock); - spin_lock(&server->srv_lock); - if (server->tcpStatus == CifsExiting) { - spin_unlock(&server->srv_lock); - return -ENOENT; - } - spin_unlock(&server->srv_lock); - - spin_lock(&server->req_lock); scredits = server->credits; /* can deadlock with reopen */ if (scredits <= 8) { diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c index 0474d0bba0a2..f280502a2aee 100644 --- a/fs/smb/client/transport.c +++ b/fs/smb/client/transport.c @@ -522,6 +522,16 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits, } while (1) { + spin_unlock(&server->req_lock); + + spin_lock(&server->srv_lock); + if (server->tcpStatus == CifsExiting) { + spin_unlock(&server->srv_lock); + return -ENOENT; + } + spin_unlock(&server->srv_lock); + + spin_lock(&server->req_lock); if (*credits < num_credits) { scredits = *credits; spin_unlock(&server->req_lock); @@ -547,15 +557,6 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits, return -ERESTARTSYS; spin_lock(&server->req_lock); } else { - spin_unlock(&server->req_lock); - - spin_lock(&server->srv_lock); - if (server->tcpStatus == CifsExiting) { - spin_unlock(&server->srv_lock); - return -ENOENT; - } - spin_unlock(&server->srv_lock); - /* * For normal commands, reserve the last MAX_COMPOUND * credits to compound requests. @@ -569,7 +570,6 @@ wait_for_free_credits(struct TCP_Server_Info *server, const int num_credits, * for servers that are slow to hand out credits on * new sessions. */ - spin_lock(&server->req_lock); if (!optype && num_credits == 1 && server->in_flight > 2 * MAX_COMPOUND && *credits <= MAX_COMPOUND) { -- cgit v1.2.3 From 99f280700b4cc02d5f141b8d15f8e9fad0418f65 Mon Sep 17 00:00:00 2001 From: Winston Wen Date: Mon, 26 Jun 2023 11:42:56 +0800 Subject: cifs: fix session state check in reconnect to avoid use-after-free issue Don't collect exiting session in smb2_reconnect_server(), because it will be released soon. Note that the exiting session will stay in server->smb_ses_list until it complete the cifs_free_ipc() and logoff() and then delete itself from the list. Signed-off-by: Winston Wen Reviewed-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/smb2pdu.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 17fe212ab895..e04766fe6f80 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -3797,6 +3797,12 @@ void smb2_reconnect_server(struct work_struct *work) spin_lock(&cifs_tcp_ses_lock); list_for_each_entry(ses, &pserver->smb_ses_list, smb_ses_list) { + spin_lock(&ses->ses_lock); + if (ses->ses_status == SES_EXITING) { + spin_unlock(&ses->ses_lock); + continue; + } + spin_unlock(&ses->ses_lock); tcon_selected = false; -- cgit v1.2.3 From 66be5c48ee1b5b8c919cc329fe6d32e16badaa40 Mon Sep 17 00:00:00 2001 From: Winston Wen Date: Mon, 26 Jun 2023 11:42:57 +0800 Subject: cifs: fix session state check in smb2_find_smb_ses Chech the session state and skip it if it's exiting. Signed-off-by: Winston Wen Reviewed-by: Shyam Prasad N Cc: stable@vger.kernel.org Signed-off-by: Steve French --- fs/smb/client/smb2transport.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c index 790acf65a092..22954a9c7a6c 100644 --- a/fs/smb/client/smb2transport.c +++ b/fs/smb/client/smb2transport.c @@ -153,7 +153,14 @@ smb2_find_smb_ses_unlocked(struct TCP_Server_Info *server, __u64 ses_id) list_for_each_entry(ses, &pserver->smb_ses_list, smb_ses_list) { if (ses->Suid != ses_id) continue; + + spin_lock(&ses->ses_lock); + if (ses->ses_status == SES_EXITING) { + spin_unlock(&ses->ses_lock); + continue; + } ++ses->ses_count; + spin_unlock(&ses->ses_lock); return ses; } -- cgit v1.2.3 From 380958ac4f93cca18b0d5775b4682ad1dff87f79 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Tue, 27 Jun 2023 12:09:43 +0000 Subject: cifs: print client_guid in DebugData Having the ClientGUID info makes it easier to debug issues related to a client on a server that serves a number of clients. This change prints the ClientGUID in DebugData. Signed-off-by: Shyam Prasad N Acked-by: Tom Talpey Signed-off-by: Steve French --- fs/smb/client/cifs_debug.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/smb/client') diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index b279f745466e..bfa8950547e2 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -330,6 +330,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) spin_lock(&server->srv_lock); if (server->hostname) seq_printf(m, "Hostname: %s ", server->hostname); + seq_printf(m, "\nClientGUID: %pUL", server->client_guid); spin_unlock(&server->srv_lock); #ifdef CONFIG_CIFS_SMB_DIRECT if (!server->rdma) -- cgit v1.2.3 From d439b29057e26464120fc6c18f97433aa003b5fe Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Tue, 27 Jun 2023 21:24:49 -0300 Subject: smb: client: fix broken file attrs with nodfs mounts *_get_inode_info() functions expect -EREMOTE when query path info calls find a DFS link, regardless whether !CONFIG_CIFS_DFS_UPCALL or 'nodfs' mount option. Otherwise, those files will miss the fake DFS file attributes. Before patch $ mount.cifs //srv/dfs /mnt/1 -o ...,nodfs $ ls -l /mnt/1 ls: cannot access '/mnt/1/link': Operation not supported total 0 -rwxr-xr-x 1 root root 0 Jul 26 2022 dfstest2_file1.txt drwxr-xr-x 2 root root 0 Aug 8 2022 dir1 d????????? ? ? ? ? ? link After patch $ mount.cifs //srv/dfs /mnt/1 -o ...,nodfs $ ls -l /mnt/1 total 0 -rwxr-xr-x 1 root root 0 Jul 26 2022 dfstest2_file1.txt drwxr-xr-x 2 root root 0 Aug 8 2022 dir1 drwx--x--x 2 root root 0 Jun 26 20:29 link Fixes: c877ce47e137 ("cifs: reduce roundtrips on create/qinfo requests") Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/smb2inode.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index 7e3ac4cb4efa..8e696fbd72fa 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -609,9 +609,6 @@ int smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, 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; } out: -- cgit v1.2.3 From 49024ec8795ed2bd7217c249ef50a70c4e25d662 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Tue, 27 Jun 2023 21:24:47 -0300 Subject: smb: client: fix parsing of source mount option Handle trailing and leading separators when parsing UNC and prefix paths in smb3_parse_devname(). Then, store the sanitised paths in smb3_fs_context::source. This fixes the following cases $ mount //srv/share// /mnt/1 -o ... $ cat /mnt/1/d0/f0 cat: /mnt/1/d0/f0: Invalid argument The -EINVAL was returned because the client sent SMB2_CREATE "\\d0\f0" rather than SMB2_CREATE "\d0\f0". $ mount //srv//share /mnt/1 -o ... mount: Invalid argument The -EINVAL was returned correctly although the client only realised it after sending a couple of bad requests rather than bailing out earlier when parsing mount options. Signed-off-by: Paulo Alcantara (SUSE) Cc: stable@vger.kernel.org Signed-off-by: Steve French --- fs/smb/client/cifs_dfs_ref.c | 20 ++++++++++----- fs/smb/client/cifsproto.h | 2 ++ fs/smb/client/dfs.c | 38 +++------------------------- fs/smb/client/fs_context.c | 59 +++++++++++++++++++++++++++++++++++++------- fs/smb/client/misc.c | 17 ++++++++----- 5 files changed, 80 insertions(+), 56 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/cifs_dfs_ref.c b/fs/smb/client/cifs_dfs_ref.c index 0329a907bdfe..b1c2499b1c3b 100644 --- a/fs/smb/client/cifs_dfs_ref.c +++ b/fs/smb/client/cifs_dfs_ref.c @@ -118,12 +118,12 @@ cifs_build_devname(char *nodename, const char *prepath) return dev; } -static int set_dest_addr(struct smb3_fs_context *ctx, const char *full_path) +static int set_dest_addr(struct smb3_fs_context *ctx) { struct sockaddr *addr = (struct sockaddr *)&ctx->dstaddr; int rc; - rc = dns_resolve_server_name_to_ip(full_path, addr, NULL); + rc = dns_resolve_server_name_to_ip(ctx->source, addr, NULL); if (!rc) cifs_set_port(addr, ctx->port); return rc; @@ -171,10 +171,9 @@ static struct vfsmount *cifs_dfs_do_automount(struct path *path) mnt = ERR_CAST(full_path); goto out; } - cifs_dbg(FYI, "%s: full_path: %s\n", __func__, full_path); tmp = *cur_ctx; - tmp.source = full_path; + tmp.source = NULL; tmp.leaf_fullpath = NULL; tmp.UNC = tmp.prepath = NULL; tmp.dfs_root_ses = NULL; @@ -185,13 +184,22 @@ static struct vfsmount *cifs_dfs_do_automount(struct path *path) goto out; } - rc = set_dest_addr(ctx, full_path); + rc = smb3_parse_devname(full_path, ctx); if (rc) { mnt = ERR_PTR(rc); goto out; } - rc = smb3_parse_devname(full_path, ctx); + ctx->source = smb3_fs_context_fullpath(ctx, '/'); + if (IS_ERR(ctx->source)) { + mnt = ERR_CAST(ctx->source); + ctx->source = NULL; + goto out; + } + cifs_dbg(FYI, "%s: ctx: source=%s UNC=%s prepath=%s dstaddr=%pISpc\n", + __func__, ctx->source, ctx->UNC, ctx->prepath, &ctx->dstaddr); + + rc = set_dest_addr(ctx); if (!rc) mnt = fc_mount(fc); else diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index d127aded2f28..293c54867d94 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -85,6 +85,8 @@ extern void release_mid(struct mid_q_entry *mid); extern void cifs_wake_up_task(struct mid_q_entry *mid); extern int cifs_handle_standard(struct TCP_Server_Info *server, struct mid_q_entry *mid); +extern char *smb3_fs_context_fullpath(const struct smb3_fs_context *ctx, + char dirsep); extern int smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx); extern int smb3_parse_opt(const char *options, const char *key, char **val); extern int cifs_ipaddr_cmp(struct sockaddr *srcaddr, struct sockaddr *rhs); diff --git a/fs/smb/client/dfs.c b/fs/smb/client/dfs.c index 2390b2fedd6a..d741f396c527 100644 --- a/fs/smb/client/dfs.c +++ b/fs/smb/client/dfs.c @@ -54,39 +54,6 @@ out: return rc; } -/* - * cifs_build_path_to_root returns full path to root when we do not have an - * existing connection (tcon) - */ -static char *build_unc_path_to_root(const struct smb3_fs_context *ctx, - const struct cifs_sb_info *cifs_sb, bool useppath) -{ - char *full_path, *pos; - unsigned int pplen = useppath && ctx->prepath ? strlen(ctx->prepath) + 1 : 0; - unsigned int unc_len = strnlen(ctx->UNC, MAX_TREE_SIZE + 1); - - if (unc_len > MAX_TREE_SIZE) - return ERR_PTR(-EINVAL); - - full_path = kmalloc(unc_len + pplen + 1, GFP_KERNEL); - if (full_path == NULL) - return ERR_PTR(-ENOMEM); - - memcpy(full_path, ctx->UNC, unc_len); - pos = full_path + unc_len; - - if (pplen) { - *pos = CIFS_DIR_SEP(cifs_sb); - memcpy(pos + 1, ctx->prepath, pplen); - pos += pplen; - } - - *pos = '\0'; /* add trailing null */ - convert_delimiter(full_path, CIFS_DIR_SEP(cifs_sb)); - cifs_dbg(FYI, "%s: full_path=%s\n", __func__, full_path); - return full_path; -} - static int get_session(struct cifs_mount_ctx *mnt_ctx, const char *full_path) { struct smb3_fs_context *ctx = mnt_ctx->fs_ctx; @@ -179,6 +146,7 @@ static int __dfs_mount_share(struct cifs_mount_ctx *mnt_ctx) struct TCP_Server_Info *server; struct cifs_tcon *tcon; char *origin_fullpath = NULL; + char sep = CIFS_DIR_SEP(cifs_sb); int num_links = 0; int rc; @@ -186,7 +154,7 @@ static int __dfs_mount_share(struct cifs_mount_ctx *mnt_ctx) if (IS_ERR(ref_path)) return PTR_ERR(ref_path); - full_path = build_unc_path_to_root(ctx, cifs_sb, true); + full_path = smb3_fs_context_fullpath(ctx, sep); if (IS_ERR(full_path)) { rc = PTR_ERR(full_path); full_path = NULL; @@ -228,7 +196,7 @@ static int __dfs_mount_share(struct cifs_mount_ctx *mnt_ctx) kfree(full_path); ref_path = full_path = NULL; - full_path = build_unc_path_to_root(ctx, cifs_sb, true); + full_path = smb3_fs_context_fullpath(ctx, sep); if (IS_ERR(full_path)) { rc = PTR_ERR(full_path); full_path = NULL; diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 1bda75609b64..4946a0c59600 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -441,14 +441,17 @@ out: * but there are some bugs that prevent rename from working if there are * multiple delimiters. * - * Returns a sanitized duplicate of @path. @gfp indicates the GFP_* flags - * for kstrdup. + * Return a sanitized duplicate of @path or NULL for empty prefix paths. + * Otherwise, return ERR_PTR. + * + * @gfp indicates the GFP_* flags for kstrdup. * The caller is responsible for freeing the original. */ #define IS_DELIM(c) ((c) == '/' || (c) == '\\') char *cifs_sanitize_prepath(char *prepath, gfp_t gfp) { char *cursor1 = prepath, *cursor2 = prepath; + char *s; /* skip all prepended delimiters */ while (IS_DELIM(*cursor1)) @@ -469,8 +472,39 @@ char *cifs_sanitize_prepath(char *prepath, gfp_t gfp) if (IS_DELIM(*(cursor2 - 1))) cursor2--; - *(cursor2) = '\0'; - return kstrdup(prepath, gfp); + *cursor2 = '\0'; + if (!*prepath) + return NULL; + s = kstrdup(prepath, gfp); + if (!s) + return ERR_PTR(-ENOMEM); + return s; +} + +/* + * Return full path based on the values of @ctx->{UNC,prepath}. + * + * It is assumed that both values were already parsed by smb3_parse_devname(). + */ +char *smb3_fs_context_fullpath(const struct smb3_fs_context *ctx, char dirsep) +{ + size_t ulen, plen; + char *s; + + ulen = strlen(ctx->UNC); + plen = ctx->prepath ? strlen(ctx->prepath) + 1 : 0; + + s = kmalloc(ulen + plen + 1, GFP_KERNEL); + if (!s) + return ERR_PTR(-ENOMEM); + memcpy(s, ctx->UNC, ulen); + if (plen) { + s[ulen] = dirsep; + memcpy(s + ulen + 1, ctx->prepath, plen); + } + s[ulen + plen] = '\0'; + convert_delimiter(s, dirsep); + return s; } /* @@ -484,6 +518,7 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx) char *pos; const char *delims = "/\\"; size_t len; + int rc; if (unlikely(!devname || !*devname)) { cifs_dbg(VFS, "Device name not specified\n"); @@ -511,6 +546,8 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx) /* now go until next delimiter or end of string */ len = strcspn(pos, delims); + if (!len) + return -EINVAL; /* move "pos" up to delimiter or NULL */ pos += len; @@ -533,8 +570,11 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx) return 0; ctx->prepath = cifs_sanitize_prepath(pos, GFP_KERNEL); - if (!ctx->prepath) - return -ENOMEM; + if (IS_ERR(ctx->prepath)) { + rc = PTR_ERR(ctx->prepath); + ctx->prepath = NULL; + return rc; + } return 0; } @@ -1146,12 +1186,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, cifs_errorf(fc, "Unknown error parsing devname\n"); goto cifs_parse_mount_err; } - ctx->source = kstrdup(param->string, GFP_KERNEL); - if (ctx->source == NULL) { + ctx->source = smb3_fs_context_fullpath(ctx, '/'); + if (IS_ERR(ctx->source)) { + ctx->source = NULL; cifs_errorf(fc, "OOM when copying UNC string\n"); goto cifs_parse_mount_err; } - fc->source = kstrdup(param->string, GFP_KERNEL); + fc->source = kstrdup(ctx->source, GFP_KERNEL); if (fc->source == NULL) { cifs_errorf(fc, "OOM when copying UNC string\n"); goto cifs_parse_mount_err; diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index cd914be905b2..609d0c0d9eca 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -1198,16 +1198,21 @@ int match_target_ip(struct TCP_Server_Info *server, int cifs_update_super_prepath(struct cifs_sb_info *cifs_sb, char *prefix) { + int rc; + kfree(cifs_sb->prepath); + cifs_sb->prepath = NULL; if (prefix && *prefix) { cifs_sb->prepath = cifs_sanitize_prepath(prefix, GFP_ATOMIC); - if (!cifs_sb->prepath) - return -ENOMEM; - - convert_delimiter(cifs_sb->prepath, CIFS_DIR_SEP(cifs_sb)); - } else - cifs_sb->prepath = NULL; + if (IS_ERR(cifs_sb->prepath)) { + rc = PTR_ERR(cifs_sb->prepath); + cifs_sb->prepath = NULL; + return rc; + } + if (cifs_sb->prepath) + convert_delimiter(cifs_sb->prepath, CIFS_DIR_SEP(cifs_sb)); + } cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_USE_PREFIX_PATH; return 0; -- cgit v1.2.3 From 3ae872de410751fe5e629e04da491a632d95201c Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Mon, 26 Jun 2023 16:04:17 -0300 Subject: smb: client: fix shared DFS root mounts with different prefixes When having two DFS root mounts that are connected to same namespace, same mount options but different prefix paths, we can't really use the shared @server->origin_fullpath when chasing DFS links in them. Move the origin_fullpath field to cifs_tcon structure so when having shared DFS root mounts with different prefix paths, and we need to chase any DFS links, dfs_get_automount_devname() will pick up the correct full path out of the @tcon that will be used for the new mount. Before patch mount.cifs //dom/dfs/dir /mnt/1 -o ... mount.cifs //dom/dfs /mnt/2 -o ... # shared server, ses, tcon # server: origin_fullpath=//dom/dfs/dir # @server->origin_fullpath + '/dir/link1' $ ls /mnt/2/dir/link1 ls: cannot open directory '/mnt/2/dir/link1': No such file or directory After patch mount.cifs //dom/dfs/dir /mnt/1 -o ... mount.cifs //dom/dfs /mnt/2 -o ... # shared server & ses # tcon_1: origin_fullpath=//dom/dfs/dir # tcon_2: origin_fullpath=//dom/dfs # @tcon_2->origin_fullpath + '/dir/link1' $ ls /mnt/2/dir/link1 dir0 dir1 dir10 dir3 dir5 dir6 dir7 dir9 target2_file.txt tsub Fixes: 8e3554150d6c ("cifs: fix sharing of DFS connections") Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/cifs_debug.c | 16 ++++++----- fs/smb/client/cifsglob.h | 10 +++---- fs/smb/client/cifsproto.h | 2 +- fs/smb/client/connect.c | 70 ++++++++++++++++++++++++++-------------------- fs/smb/client/dfs.c | 55 ++++++++++++++---------------------- fs/smb/client/dfs.h | 19 ++++++------- fs/smb/client/dfs_cache.c | 8 ++++-- fs/smb/client/misc.c | 38 +++++++++++++++++++------ 8 files changed, 118 insertions(+), 100 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index bfa8950547e2..fb4162a52844 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -122,6 +122,12 @@ static void cifs_debug_tcon(struct seq_file *m, struct cifs_tcon *tcon) seq_puts(m, " nosparse"); if (tcon->need_reconnect) seq_puts(m, "\tDISCONNECTED "); + spin_lock(&tcon->tc_lock); + if (tcon->origin_fullpath) { + seq_printf(m, "\n\tDFS origin fullpath: %s", + tcon->origin_fullpath); + } + spin_unlock(&tcon->tc_lock); seq_putc(m, '\n'); } @@ -428,13 +434,9 @@ skip_rdma: seq_printf(m, "\nIn Send: %d In MaxReq Wait: %d", atomic_read(&server->in_send), atomic_read(&server->num_waiters)); - if (IS_ENABLED(CONFIG_CIFS_DFS_UPCALL)) { - if (server->origin_fullpath) - seq_printf(m, "\nDFS origin full path: %s", - server->origin_fullpath); - if (server->leaf_fullpath) - seq_printf(m, "\nDFS leaf full path: %s", - server->leaf_fullpath); + if (server->leaf_fullpath) { + seq_printf(m, "\nDFS leaf full path: %s", + server->leaf_fullpath); } seq_printf(m, "\n\n\tSessions: "); diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index b212a4e16b39..ca2da713c5fe 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -736,23 +736,20 @@ struct TCP_Server_Info { #endif struct mutex refpath_lock; /* protects leaf_fullpath */ /* - * origin_fullpath: Canonical copy of smb3_fs_context::source. - * It is used for matching existing DFS tcons. - * * leaf_fullpath: Canonical DFS referral path related to this * connection. * It is used in DFS cache refresher, reconnect and may * change due to nested DFS links. * - * Both protected by @refpath_lock and @srv_lock. The @refpath_lock is - * mosly used for not requiring a copy of @leaf_fullpath when getting + * Protected by @refpath_lock and @srv_lock. The @refpath_lock is + * mostly used for not requiring a copy of @leaf_fullpath when getting * cached or new DFS referrals (which might also sleep during I/O). * While @srv_lock is held for making string and NULL comparions against * both fields as in mount(2) and cache refresh. * * format: \\HOST\SHARE[\OPTIONAL PATH] */ - char *origin_fullpath, *leaf_fullpath; + char *leaf_fullpath; }; static inline bool is_smb1(struct TCP_Server_Info *server) @@ -1205,6 +1202,7 @@ struct cifs_tcon { struct delayed_work dfs_cache_work; #endif struct delayed_work query_interfaces; /* query interfaces workqueue job */ + char *origin_fullpath; /* canonical copy of smb3_fs_context::source */ }; /* diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index 293c54867d94..1d71d658e167 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -652,7 +652,7 @@ int smb2_parse_query_directory(struct cifs_tcon *tcon, struct kvec *rsp_iov, int resp_buftype, struct cifs_search_info *srch_inf); -struct super_block *cifs_get_tcp_super(struct TCP_Server_Info *server); +struct super_block *cifs_get_dfs_tcon_super(struct cifs_tcon *tcon); void cifs_put_tcp_super(struct super_block *sb); int cifs_update_super_prepath(struct cifs_sb_info *cifs_sb, char *prefix); char *extract_hostname(const char *unc); diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 972bc0804054..dab7bc876507 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -996,7 +996,6 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server) */ } - kfree(server->origin_fullpath); kfree(server->leaf_fullpath); kfree(server); @@ -1436,7 +1435,9 @@ match_security(struct TCP_Server_Info *server, struct smb3_fs_context *ctx) } /* this function must be called with srv_lock held */ -static int match_server(struct TCP_Server_Info *server, struct smb3_fs_context *ctx) +static int match_server(struct TCP_Server_Info *server, + struct smb3_fs_context *ctx, + bool match_super) { struct sockaddr *addr = (struct sockaddr *)&ctx->dstaddr; @@ -1467,36 +1468,38 @@ static int match_server(struct TCP_Server_Info *server, struct smb3_fs_context * (struct sockaddr *)&server->srcaddr)) return 0; /* - * - Match for an DFS tcon (@server->origin_fullpath). - * - Match for an DFS root server connection (@server->leaf_fullpath). - * - If none of the above and @ctx->leaf_fullpath is set, then - * it is a new DFS connection. - * - If 'nodfs' mount option was passed, then match only connections - * that have no DFS referrals set - * (e.g. can't failover to other targets). + * When matching cifs.ko superblocks (@match_super == true), we can't + * really match either @server->leaf_fullpath or @server->dstaddr + * directly since this @server might belong to a completely different + * server -- in case of domain-based DFS referrals or DFS links -- as + * provided earlier by mount(2) through 'source' and 'ip' options. + * + * Otherwise, match the DFS referral in @server->leaf_fullpath or the + * destination address in @server->dstaddr. + * + * When using 'nodfs' mount option, we avoid sharing it with DFS + * connections as they might failover. */ - if (!ctx->nodfs) { - if (ctx->source && server->origin_fullpath) { - if (!dfs_src_pathname_equal(ctx->source, - server->origin_fullpath)) + if (!match_super) { + if (!ctx->nodfs) { + if (server->leaf_fullpath) { + if (!ctx->leaf_fullpath || + strcasecmp(server->leaf_fullpath, + ctx->leaf_fullpath)) + return 0; + } else if (ctx->leaf_fullpath) { return 0; + } } else if (server->leaf_fullpath) { - if (!ctx->leaf_fullpath || - strcasecmp(server->leaf_fullpath, - ctx->leaf_fullpath)) - return 0; - } else if (ctx->leaf_fullpath) { return 0; } - } else if (server->origin_fullpath || server->leaf_fullpath) { - return 0; } /* * Match for a regular connection (address/hostname/port) which has no * DFS referrals set. */ - if (!server->origin_fullpath && !server->leaf_fullpath && + if (!server->leaf_fullpath && (strcasecmp(server->hostname, ctx->server_hostname) || !match_server_address(server, addr) || !match_port(server, addr))) @@ -1532,7 +1535,8 @@ cifs_find_tcp_session(struct smb3_fs_context *ctx) * Skip ses channels since they're only handled in lower layers * (e.g. cifs_send_recv). */ - if (CIFS_SERVER_IS_CHAN(server) || !match_server(server, ctx)) { + if (CIFS_SERVER_IS_CHAN(server) || + !match_server(server, ctx, false)) { spin_unlock(&server->srv_lock); continue; } @@ -2320,10 +2324,16 @@ static int match_tcon(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) if (tcon->status == TID_EXITING) return 0; - /* Skip UNC validation when matching DFS connections or superblocks */ - if (!server->origin_fullpath && !server->leaf_fullpath && - strncmp(tcon->tree_name, ctx->UNC, MAX_TREE_SIZE)) + + if (tcon->origin_fullpath) { + if (!ctx->source || + !dfs_src_pathname_equal(ctx->source, + tcon->origin_fullpath)) + return 0; + } else if (!server->leaf_fullpath && + strncmp(tcon->tree_name, ctx->UNC, MAX_TREE_SIZE)) { return 0; + } if (tcon->seal != ctx->seal) return 0; if (tcon->snapshot_time != ctx->snapshot_time) @@ -2722,7 +2732,7 @@ compare_mount_options(struct super_block *sb, struct cifs_mnt_data *mnt_data) } static int match_prepath(struct super_block *sb, - struct TCP_Server_Info *server, + struct cifs_tcon *tcon, struct cifs_mnt_data *mnt_data) { struct smb3_fs_context *ctx = mnt_data->ctx; @@ -2733,8 +2743,8 @@ static int match_prepath(struct super_block *sb, bool new_set = (new->mnt_cifs_flags & CIFS_MOUNT_USE_PREFIX_PATH) && new->prepath; - if (server->origin_fullpath && - dfs_src_pathname_equal(server->origin_fullpath, ctx->source)) + if (tcon->origin_fullpath && + dfs_src_pathname_equal(tcon->origin_fullpath, ctx->source)) return 1; if (old_set && new_set && !strcmp(new->prepath, old->prepath)) @@ -2783,10 +2793,10 @@ cifs_match_super(struct super_block *sb, void *data) spin_lock(&ses->ses_lock); spin_lock(&ses->chan_lock); spin_lock(&tcon->tc_lock); - if (!match_server(tcp_srv, ctx) || + if (!match_server(tcp_srv, ctx, true) || !match_session(ses, ctx) || !match_tcon(tcon, ctx) || - !match_prepath(sb, tcp_srv, mnt_data)) { + !match_prepath(sb, tcon, mnt_data)) { rc = 0; goto out; } diff --git a/fs/smb/client/dfs.c b/fs/smb/client/dfs.c index d741f396c527..dd06b8a0ff7a 100644 --- a/fs/smb/client/dfs.c +++ b/fs/smb/client/dfs.c @@ -217,14 +217,12 @@ static int __dfs_mount_share(struct cifs_mount_ctx *mnt_ctx) server = mnt_ctx->server; tcon = mnt_ctx->tcon; - mutex_lock(&server->refpath_lock); - spin_lock(&server->srv_lock); - if (!server->origin_fullpath) { - server->origin_fullpath = origin_fullpath; + spin_lock(&tcon->tc_lock); + if (!tcon->origin_fullpath) { + tcon->origin_fullpath = origin_fullpath; origin_fullpath = NULL; } - spin_unlock(&server->srv_lock); - mutex_unlock(&server->refpath_lock); + spin_unlock(&tcon->tc_lock); if (list_empty(&tcon->dfs_ses_list)) { list_replace_init(&mnt_ctx->dfs_ses_list, @@ -247,18 +245,13 @@ int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs) { struct smb3_fs_context *ctx = mnt_ctx->fs_ctx; struct cifs_ses *ses; - char *source = ctx->source; bool nodfs = ctx->nodfs; int rc; *isdfs = false; - /* Temporarily set @ctx->source to NULL as we're not matching DFS - * superblocks yet. See cifs_match_super() and match_server(). - */ - ctx->source = NULL; rc = get_session(mnt_ctx, NULL); if (rc) - goto out; + return rc; ctx->dfs_root_ses = mnt_ctx->ses; /* @@ -272,7 +265,7 @@ int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs) rc = dfs_get_referral(mnt_ctx, ctx->UNC + 1, NULL, NULL); if (rc) { if (rc != -ENOENT && rc != -EOPNOTSUPP && rc != -EIO) - goto out; + return rc; nodfs = true; } } @@ -280,7 +273,7 @@ int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs) rc = cifs_mount_get_tcon(mnt_ctx); if (!rc) rc = cifs_is_path_remote(mnt_ctx); - goto out; + return rc; } *isdfs = true; @@ -296,12 +289,7 @@ int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs) rc = __dfs_mount_share(mnt_ctx); if (ses == ctx->dfs_root_ses) cifs_put_smb_ses(ses); -out: - /* - * Restore previous value of @ctx->source so DFS superblock can be - * matched in cifs_match_super(). - */ - ctx->source = source; + return rc; } @@ -535,11 +523,11 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru int rc; struct TCP_Server_Info *server = tcon->ses->server; const struct smb_version_operations *ops = server->ops; - struct super_block *sb = NULL; - struct cifs_sb_info *cifs_sb; struct dfs_cache_tgt_list tl = DFS_CACHE_TGT_LIST_INIT(tl); - char *tree; + struct cifs_sb_info *cifs_sb = NULL; + struct super_block *sb = NULL; struct dfs_info3_param ref = {0}; + char *tree; /* only send once per connect */ spin_lock(&tcon->tc_lock); @@ -571,19 +559,18 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru goto out; } - sb = cifs_get_tcp_super(server); - if (IS_ERR(sb)) { - rc = PTR_ERR(sb); - cifs_dbg(VFS, "%s: could not find superblock: %d\n", __func__, rc); - goto out; - } - - cifs_sb = CIFS_SB(sb); + sb = cifs_get_dfs_tcon_super(tcon); + if (!IS_ERR(sb)) + cifs_sb = CIFS_SB(sb); - /* If it is not dfs or there was no cached dfs referral, then reconnect to same share */ - if (!server->leaf_fullpath || + /* + * Tree connect to last share in @tcon->tree_name whether dfs super or + * cached dfs referral was not found. + */ + if (!cifs_sb || !server->leaf_fullpath || dfs_cache_noreq_find(server->leaf_fullpath + 1, &ref, &tl)) { - rc = ops->tree_connect(xid, tcon->ses, tcon->tree_name, tcon, cifs_sb->local_nls); + rc = ops->tree_connect(xid, tcon->ses, tcon->tree_name, tcon, + cifs_sb ? cifs_sb->local_nls : nlsc); goto out; } diff --git a/fs/smb/client/dfs.h b/fs/smb/client/dfs.h index 1c90df5ecfbd..98e9d2aca6a7 100644 --- a/fs/smb/client/dfs.h +++ b/fs/smb/client/dfs.h @@ -39,16 +39,15 @@ static inline char *dfs_get_automount_devname(struct dentry *dentry, void *page) { struct cifs_sb_info *cifs_sb = CIFS_SB(dentry->d_sb); struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); - struct TCP_Server_Info *server = tcon->ses->server; size_t len; char *s; - spin_lock(&server->srv_lock); - if (unlikely(!server->origin_fullpath)) { - spin_unlock(&server->srv_lock); + spin_lock(&tcon->tc_lock); + if (unlikely(!tcon->origin_fullpath)) { + spin_unlock(&tcon->tc_lock); return ERR_PTR(-EREMOTE); } - spin_unlock(&server->srv_lock); + spin_unlock(&tcon->tc_lock); s = dentry_path_raw(dentry, page, PATH_MAX); if (IS_ERR(s)) @@ -57,16 +56,16 @@ static inline char *dfs_get_automount_devname(struct dentry *dentry, void *page) if (!s[1]) s++; - spin_lock(&server->srv_lock); - len = strlen(server->origin_fullpath); + spin_lock(&tcon->tc_lock); + len = strlen(tcon->origin_fullpath); if (s < (char *)page + len) { - spin_unlock(&server->srv_lock); + spin_unlock(&tcon->tc_lock); return ERR_PTR(-ENAMETOOLONG); } s -= len; - memcpy(s, server->origin_fullpath, len); - spin_unlock(&server->srv_lock); + memcpy(s, tcon->origin_fullpath, len); + spin_unlock(&tcon->tc_lock); convert_delimiter(s, '/'); return s; diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c index 1513b2709889..33adf43a01f1 100644 --- a/fs/smb/client/dfs_cache.c +++ b/fs/smb/client/dfs_cache.c @@ -1248,18 +1248,20 @@ static int refresh_tcon(struct cifs_tcon *tcon, bool force_refresh) int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb) { struct cifs_tcon *tcon; - struct TCP_Server_Info *server; if (!cifs_sb || !cifs_sb->master_tlink) return -EINVAL; tcon = cifs_sb_master_tcon(cifs_sb); - server = tcon->ses->server; - if (!server->origin_fullpath) { + spin_lock(&tcon->tc_lock); + if (!tcon->origin_fullpath) { + spin_unlock(&tcon->tc_lock); cifs_dbg(FYI, "%s: not a dfs mount\n", __func__); return 0; } + spin_unlock(&tcon->tc_lock); + /* * After reconnecting to a different server, unique ids won't match anymore, so we disable * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE). diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index 609d0c0d9eca..70dbfe6584f9 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -156,6 +156,7 @@ tconInfoFree(struct cifs_tcon *tcon) #ifdef CONFIG_CIFS_DFS_UPCALL dfs_put_root_smb_sessions(&tcon->dfs_ses_list); #endif + kfree(tcon->origin_fullpath); kfree(tcon); } @@ -1106,20 +1107,25 @@ struct super_cb_data { struct super_block *sb; }; -static void tcp_super_cb(struct super_block *sb, void *arg) +static void tcon_super_cb(struct super_block *sb, void *arg) { struct super_cb_data *sd = arg; - struct TCP_Server_Info *server = sd->data; struct cifs_sb_info *cifs_sb; - struct cifs_tcon *tcon; + struct cifs_tcon *t1 = sd->data, *t2; if (sd->sb) return; cifs_sb = CIFS_SB(sb); - tcon = cifs_sb_master_tcon(cifs_sb); - if (tcon->ses->server == server) + t2 = cifs_sb_master_tcon(cifs_sb); + + spin_lock(&t2->tc_lock); + if (t1->ses == t2->ses && + t1->ses->server == t2->ses->server && + t2->origin_fullpath && + dfs_src_pathname_equal(t2->origin_fullpath, t1->origin_fullpath)) sd->sb = sb; + spin_unlock(&t2->tc_lock); } static struct super_block *__cifs_get_super(void (*f)(struct super_block *, void *), @@ -1145,6 +1151,7 @@ static struct super_block *__cifs_get_super(void (*f)(struct super_block *, void return sd.sb; } } + pr_warn_once("%s: could not find dfs superblock\n", __func__); return ERR_PTR(-EINVAL); } @@ -1154,9 +1161,15 @@ static void __cifs_put_super(struct super_block *sb) cifs_sb_deactive(sb); } -struct super_block *cifs_get_tcp_super(struct TCP_Server_Info *server) +struct super_block *cifs_get_dfs_tcon_super(struct cifs_tcon *tcon) { - return __cifs_get_super(tcp_super_cb, server); + spin_lock(&tcon->tc_lock); + if (!tcon->origin_fullpath) { + spin_unlock(&tcon->tc_lock); + return ERR_PTR(-ENOENT); + } + spin_unlock(&tcon->tc_lock); + return __cifs_get_super(tcon_super_cb, tcon); } void cifs_put_tcp_super(struct super_block *sb) @@ -1243,9 +1256,16 @@ int cifs_inval_name_dfs_link_error(const unsigned int xid, */ if (strlen(full_path) < 2 || !cifs_sb || (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS) || - !is_tcon_dfs(tcon) || !ses->server->origin_fullpath) + !is_tcon_dfs(tcon)) return 0; + spin_lock(&tcon->tc_lock); + if (!tcon->origin_fullpath) { + spin_unlock(&tcon->tc_lock); + return 0; + } + spin_unlock(&tcon->tc_lock); + /* * 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. @@ -1269,7 +1289,7 @@ int cifs_inval_name_dfs_link_error(const unsigned int xid, /* * XXX: we are not using dfs_cache_find() here because we might - * end filling all the DFS cache and thus potentially + * end up filling all the DFS cache and thus potentially * removing cached DFS targets that the client would eventually * need during failover. */ -- cgit v1.2.3 From 5f2a0afa9890e728428db2ed9281bddca242e90b Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Tue, 27 Jun 2023 21:24:50 -0300 Subject: smb: client: improve DFS mount check Some servers may return error codes from REQ_GET_DFS_REFERRAL requests that are unexpected by the client, so to make it easier, assume non-DFS mounts when the client can't get the initial DFS referral of @ctx->UNC in dfs_mount_share(). Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/smb/client/dfs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/dfs.c b/fs/smb/client/dfs.c index dd06b8a0ff7a..26d14dd0482e 100644 --- a/fs/smb/client/dfs.c +++ b/fs/smb/client/dfs.c @@ -264,8 +264,9 @@ int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs) if (!nodfs) { rc = dfs_get_referral(mnt_ctx, ctx->UNC + 1, NULL, NULL); if (rc) { - if (rc != -ENOENT && rc != -EOPNOTSUPP && rc != -EIO) - return rc; + cifs_dbg(FYI, "%s: no dfs referral for %s: %d\n", + __func__, ctx->UNC + 1, rc); + cifs_dbg(FYI, "%s: assuming non-dfs mount...\n", __func__); nodfs = true; } } -- cgit v1.2.3 From ac615db03ba508d42d240612262f21f2e5836b67 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Tue, 20 Jun 2023 02:56:06 +0000 Subject: cifs: log session id when a matching ses is not found We do not log the session id in crypt_setup when a matching session is not found. Printing the session id helps debugging here. This change does just that. This change also changes this log to FYI, since it is normal to see then during a reconnect. Doing the same for a similar log in case of signed connections. The plan is to have a tracepoint for this event, so that we will be able to see this event if need be. That will be done as another change. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/smb2ops.c | 4 ++-- fs/smb/client/smb2transport.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 3696d4ce0df3..7f8e07c42d4c 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -4444,8 +4444,8 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst, rc = smb2_get_enc_key(server, le64_to_cpu(tr_hdr->SessionId), enc, key); if (rc) { - cifs_server_dbg(VFS, "%s: Could not get %scryption key\n", __func__, - enc ? "en" : "de"); + cifs_server_dbg(FYI, "%s: Could not get %scryption key. sid: 0x%llx\n", __func__, + enc ? "en" : "de", le64_to_cpu(tr_hdr->SessionId)); return rc; } diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c index 22954a9c7a6c..c3e9cb5c7be5 100644 --- a/fs/smb/client/smb2transport.c +++ b/fs/smb/client/smb2transport.c @@ -92,7 +92,7 @@ int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key) if (ses->Suid == ses_id) goto found; } - cifs_server_dbg(VFS, "%s: Could not find session 0x%llx\n", + cifs_server_dbg(FYI, "%s: Could not find session 0x%llx\n", __func__, ses_id); rc = -ENOENT; goto out; @@ -564,7 +564,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, rc = smb2_get_sign_key(le64_to_cpu(shdr->SessionId), server, key); if (unlikely(rc)) { - cifs_server_dbg(VFS, "%s: Could not get signing key\n", __func__); + cifs_server_dbg(FYI, "%s: Could not get signing key\n", __func__); return rc; } -- cgit v1.2.3 From 61986a58bc6abbb1aea26e52bd269f49e5bacf19 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Tue, 27 Jun 2023 06:22:20 +0000 Subject: cifs: new dynamic tracepoint to track ses not found errors It is perfectly valid to not find session not found errors when a reconnect of a session happens when requests for the same session are happening in parallel. We had these log messages as VFS logs. My last change dumped these logs as FYI logs. This change just creates a new dynamic tracepoint to capture events of this type, just in case it is useful while debugging issues in the future. Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/smb2ops.c | 2 ++ fs/smb/client/smb2transport.c | 1 + fs/smb/client/trace.h | 20 ++++++++++++++++++++ 3 files changed, 23 insertions(+) (limited to 'fs/smb/client') diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 7f8e07c42d4c..eb1340b9125e 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -4414,6 +4414,8 @@ smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key) } spin_unlock(&cifs_tcp_ses_lock); + trace_smb3_ses_not_found(ses_id); + return -EAGAIN; } /* diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c index c3e9cb5c7be5..c6db898dab7c 100644 --- a/fs/smb/client/smb2transport.c +++ b/fs/smb/client/smb2transport.c @@ -92,6 +92,7 @@ int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key) if (ses->Suid == ses_id) goto found; } + trace_smb3_ses_not_found(ses_id); cifs_server_dbg(FYI, "%s: Could not find session 0x%llx\n", __func__, ses_id); rc = -ENOENT; diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h index d3053bd8ae73..e671bd16f00c 100644 --- a/fs/smb/client/trace.h +++ b/fs/smb/client/trace.h @@ -1003,6 +1003,26 @@ DEFINE_EVENT(smb3_reconnect_class, smb3_##name, \ DEFINE_SMB3_RECONNECT_EVENT(reconnect); DEFINE_SMB3_RECONNECT_EVENT(partial_send_reconnect); +DECLARE_EVENT_CLASS(smb3_ses_class, + TP_PROTO(__u64 sesid), + TP_ARGS(sesid), + TP_STRUCT__entry( + __field(__u64, sesid) + ), + TP_fast_assign( + __entry->sesid = sesid; + ), + TP_printk("sid=0x%llx", + __entry->sesid) +) + +#define DEFINE_SMB3_SES_EVENT(name) \ +DEFINE_EVENT(smb3_ses_class, smb3_##name, \ + TP_PROTO(__u64 sesid), \ + TP_ARGS(sesid)) + +DEFINE_SMB3_SES_EVENT(ses_not_found); + DECLARE_EVENT_CLASS(smb3_credit_class, TP_PROTO(__u64 currmid, __u64 conn_id, -- cgit v1.2.3