From 8378a51e3f8140f60901fb27208cc7a6e47047b5 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Tue, 31 May 2022 13:01:17 +1000 Subject: cifs: fix potential double free during failed mount RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2088799 Cc: stable@vger.kernel.org Signed-off-by: Roberto Bergantinos Signed-off-by: Ronnie Sahlberg Signed-off-by: Steve French --- fs/cifs/cifsfs.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index f539a39d47f5..12c872800326 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -838,7 +838,7 @@ cifs_smb3_do_mount(struct file_system_type *fs_type, int flags, struct smb3_fs_context *old_ctx) { int rc; - struct super_block *sb; + struct super_block *sb = NULL; struct cifs_sb_info *cifs_sb = NULL; struct cifs_mnt_data mnt_data; struct dentry *root; @@ -934,9 +934,11 @@ out_super: return root; out: if (cifs_sb) { - kfree(cifs_sb->prepath); - smb3_cleanup_fs_context(cifs_sb->ctx); - kfree(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); + } } return root; } -- cgit v1.2.3 From 0d5106a80e08cbbb273f35dc373f4f19e5ec8b03 Mon Sep 17 00:00:00 2001 From: Enzo Matsumiya Date: Tue, 31 May 2022 14:27:18 -0300 Subject: cifs: remove repeated debug message on cifs_put_smb_ses() Similar message is printed a few lines later in the same function Signed-off-by: Enzo Matsumiya Signed-off-by: Steve French --- fs/cifs/connect.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 53373a3649e1..f54409f6370a 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1845,7 +1845,6 @@ void cifs_put_smb_ses(struct cifs_ses *ses) unsigned int rc, xid; unsigned int chan_count; struct TCP_Server_Info *server = ses->server; - cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count); spin_lock(&cifs_tcp_ses_lock); if (ses->ses_status == SES_EXITING) { -- cgit v1.2.3 From f66f8b94e7f2f4ac9fffe710be231ca8f25c5057 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Wed, 1 Jun 2022 08:48:38 +1000 Subject: cifs: when extending a file with falloc we should make files not-sparse as this is the only way to make sure the region is allocated. Fix the conditional that was wrong and only tried to make already non-sparse files non-sparse. Cc: stable@vger.kernel.org Signed-off-by: Ronnie Sahlberg Signed-off-by: Steve French --- fs/cifs/smb2ops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index d7ade739cde1..03ab28c341c4 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -3859,7 +3859,7 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon, if (rc) goto out; - if ((cifsi->cifsAttrs & FILE_ATTRIBUTE_SPARSE_FILE) == 0) + if (cifsi->cifsAttrs & FILE_ATTRIBUTE_SPARSE_FILE) smb2_set_sparse(xid, tcon, cfile, inode, false); eof = cpu_to_le64(off + len); -- cgit v1.2.3 From cc391b694ff085f62f133e6b8f864d43a8e69dfd Mon Sep 17 00:00:00 2001 From: Vincent Whitchurch Date: Wed, 1 Jun 2022 00:03:18 -0500 Subject: cifs: fix potential deadlock in direct reclaim The srv_mutex is used during writeback so cifs should ensure that allocations done when that mutex is held are done with GFP_NOFS, to avoid having direct reclaim ending up waiting for the same mutex and causing a deadlock. This is detected by lockdep with the splat below: ====================================================== WARNING: possible circular locking dependency detected 5.18.0 #70 Not tainted ------------------------------------------------------ kswapd0/49 is trying to acquire lock: ffff8880195782e0 (&tcp_ses->srv_mutex){+.+.}-{3:3}, at: compound_send_recv but task is already holding lock: ffffffffa98e66c0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (fs_reclaim){+.+.}-{0:0}: fs_reclaim_acquire kmem_cache_alloc_trace __request_module crypto_alg_mod_lookup crypto_alloc_tfm_node crypto_alloc_shash cifs_alloc_hash smb311_crypto_shash_allocate smb311_update_preauth_hash compound_send_recv cifs_send_recv SMB2_negotiate smb2_negotiate cifs_negotiate_protocol cifs_get_smb_ses cifs_mount cifs_smb3_do_mount smb3_get_tree vfs_get_tree path_mount __x64_sys_mount do_syscall_64 entry_SYSCALL_64_after_hwframe -> #0 (&tcp_ses->srv_mutex){+.+.}-{3:3}: __lock_acquire lock_acquire __mutex_lock mutex_lock_nested compound_send_recv cifs_send_recv SMB2_write smb2_sync_write cifs_write cifs_writepage_locked cifs_writepage shrink_page_list shrink_lruvec shrink_node balance_pgdat kswapd kthread ret_from_fork other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(fs_reclaim); lock(&tcp_ses->srv_mutex); lock(fs_reclaim); lock(&tcp_ses->srv_mutex); *** DEADLOCK *** 1 lock held by kswapd0/49: #0: ffffffffa98e66c0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat stack backtrace: CPU: 2 PID: 49 Comm: kswapd0 Not tainted 5.18.0 #70 Call Trace: dump_stack_lvl dump_stack print_circular_bug.cold check_noncircular __lock_acquire lock_acquire __mutex_lock mutex_lock_nested compound_send_recv cifs_send_recv SMB2_write smb2_sync_write cifs_write cifs_writepage_locked cifs_writepage shrink_page_list shrink_lruvec shrink_node balance_pgdat kswapd kthread ret_from_fork Fix this by using the memalloc_nofs_save/restore APIs around the places where the srv_mutex is held. Do this in a wrapper function for the lock/unlock of the srv_mutex, and rename the srv_mutex to avoid missing call sites in the conversion. Note that there is another lockdep warning involving internal crypto locks, which was masked by this problem and is visible after this fix, see the discussion in this thread: https://lore.kernel.org/all/20220523123755.GA13668@axis.com/ Link: https://lore.kernel.org/r/CANT5p=rqcYfYMVHirqvdnnca4Mo+JQSw5Qu12v=kPfpk5yhhmg@mail.gmail.com/ Reported-by: Shyam Prasad N Suggested-by: Lars Persson Reviewed-by: Ronnie Sahlberg Reviewed-by: Enzo Matsumiya Signed-off-by: Vincent Whitchurch Signed-off-by: Steve French --- fs/cifs/cifs_swn.c | 4 ++-- fs/cifs/cifsencrypt.c | 8 ++++---- fs/cifs/cifsglob.h | 20 +++++++++++++++++++- fs/cifs/connect.c | 26 +++++++++++++------------- fs/cifs/dfs_cache.c | 4 ++-- fs/cifs/sess.c | 6 +++--- fs/cifs/smb1ops.c | 6 +++--- fs/cifs/smb2pdu.c | 6 +++--- fs/cifs/smbdirect.c | 4 ++-- fs/cifs/transport.c | 40 ++++++++++++++++++++-------------------- 10 files changed, 71 insertions(+), 53 deletions(-) diff --git a/fs/cifs/cifs_swn.c b/fs/cifs/cifs_swn.c index 180c234c2f46..1e4c7cc5287f 100644 --- a/fs/cifs/cifs_swn.c +++ b/fs/cifs/cifs_swn.c @@ -465,7 +465,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a int ret = 0; /* Store the reconnect address */ - mutex_lock(&tcon->ses->server->srv_mutex); + cifs_server_lock(tcon->ses->server); if (cifs_sockaddr_equal(&tcon->ses->server->dstaddr, addr)) goto unlock; @@ -501,7 +501,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a cifs_signal_cifsd_for_reconnect(tcon->ses->server, false); unlock: - mutex_unlock(&tcon->ses->server->srv_mutex); + cifs_server_unlock(tcon->ses->server); return ret; } diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index 0912d8bbbac1..663cb9db4908 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -236,9 +236,9 @@ int cifs_verify_signature(struct smb_rqst *rqst, cpu_to_le32(expected_sequence_number); cifs_pdu->Signature.Sequence.Reserved = 0; - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = cifs_calc_signature(rqst, server, what_we_think_sig_should_be); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc) return rc; @@ -626,7 +626,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) memcpy(ses->auth_key.response + baselen, tiblob, tilen); - mutex_lock(&ses->server->srv_mutex); + cifs_server_lock(ses->server); rc = cifs_alloc_hash("hmac(md5)", &ses->server->secmech.hmacmd5, @@ -678,7 +678,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__); unlock: - mutex_unlock(&ses->server->srv_mutex); + cifs_server_unlock(ses->server); setup_ntlmv2_rsp_ret: kfree(tiblob); diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 68da230c7f11..d589e687611d 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include "cifs_fs_sb.h" #include "cifsacl.h" @@ -628,7 +629,8 @@ struct TCP_Server_Info { unsigned int in_flight; /* number of requests on the wire to server */ unsigned int max_in_flight; /* max number of requests that were on wire */ spinlock_t req_lock; /* protect the two values above */ - struct mutex srv_mutex; + struct mutex _srv_mutex; + unsigned int nofs_flag; struct task_struct *tsk; char server_GUID[16]; __u16 sec_mode; @@ -743,6 +745,22 @@ struct TCP_Server_Info { #endif }; +static inline void cifs_server_lock(struct TCP_Server_Info *server) +{ + unsigned int nofs_flag = memalloc_nofs_save(); + + mutex_lock(&server->_srv_mutex); + server->nofs_flag = nofs_flag; +} + +static inline void cifs_server_unlock(struct TCP_Server_Info *server) +{ + unsigned int nofs_flag = server->nofs_flag; + + mutex_unlock(&server->_srv_mutex); + memalloc_nofs_restore(nofs_flag); +} + struct cifs_credits { unsigned int value; unsigned int instance; diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index f54409f6370a..d46702f5a663 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -148,7 +148,7 @@ static void cifs_resolve_server(struct work_struct *work) struct TCP_Server_Info *server = container_of(work, struct TCP_Server_Info, resolve.work); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); /* * Resolve the hostname again to make sure that IP address is up-to-date. @@ -159,7 +159,7 @@ static void cifs_resolve_server(struct work_struct *work) __func__, rc); } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); } /* @@ -267,7 +267,7 @@ cifs_abort_connection(struct TCP_Server_Info *server) /* do not want to be sending data on a socket we are freeing */ cifs_dbg(FYI, "%s: tearing down socket\n", __func__); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); if (server->ssocket) { cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n", server->ssocket->state, server->ssocket->flags); @@ -296,7 +296,7 @@ cifs_abort_connection(struct TCP_Server_Info *server) mid->mid_flags |= MID_DELETED; } spin_unlock(&GlobalMid_Lock); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__); list_for_each_entry_safe(mid, nmid, &retry_list, qhead) { @@ -306,9 +306,9 @@ cifs_abort_connection(struct TCP_Server_Info *server) } if (cifs_rdma_enabled(server)) { - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); smbd_destroy(server); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); } } @@ -359,7 +359,7 @@ static int __cifs_reconnect(struct TCP_Server_Info *server, do { try_to_freeze(); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); if (!cifs_swn_set_server_dstaddr(server)) { /* resolve the hostname again to make sure that IP address is up-to-date */ @@ -372,7 +372,7 @@ static int __cifs_reconnect(struct TCP_Server_Info *server, else rc = generic_ip_connect(server); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "%s: reconnect error %d\n", __func__, rc); msleep(3000); } else { @@ -383,7 +383,7 @@ static int __cifs_reconnect(struct TCP_Server_Info *server, server->tcpStatus = CifsNeedNegotiate; spin_unlock(&cifs_tcp_ses_lock); cifs_swn_reset_server_dstaddr(server); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); mod_delayed_work(cifsiod_wq, &server->reconnect, 0); } } while (server->tcpStatus == CifsNeedReconnect); @@ -488,12 +488,12 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server) do { try_to_freeze(); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = reconnect_target_unlocked(server, &tl, &target_hint); if (rc) { /* Failed to reconnect socket */ - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "%s: reconnect error %d\n", __func__, rc); msleep(3000); continue; @@ -510,7 +510,7 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server) server->tcpStatus = CifsNeedNegotiate; spin_unlock(&cifs_tcp_ses_lock); cifs_swn_reset_server_dstaddr(server); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); mod_delayed_work(cifsiod_wq, &server->reconnect, 0); } while (server->tcpStatus == CifsNeedReconnect); @@ -1565,7 +1565,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx, init_waitqueue_head(&tcp_ses->response_q); init_waitqueue_head(&tcp_ses->request_q); INIT_LIST_HEAD(&tcp_ses->pending_mid_q); - mutex_init(&tcp_ses->srv_mutex); + mutex_init(&tcp_ses->_srv_mutex); memcpy(tcp_ses->workstation_RFC1001_name, ctx->source_rfc1001_name, RFC1001_NAME_LEN_WITH_NULL); memcpy(tcp_ses->server_RFC1001_name, diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c index c5dd6f7305bd..aa7d00b5b3e7 100644 --- a/fs/cifs/dfs_cache.c +++ b/fs/cifs/dfs_cache.c @@ -1327,9 +1327,9 @@ static bool target_share_equal(struct TCP_Server_Info *server, const char *s1, c cifs_dbg(VFS, "%s: failed to convert address \'%s\'. skip address matching.\n", __func__, ip); } else { - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); match = cifs_match_ipaddr((struct sockaddr *)&server->dstaddr, &sa); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); } kfree(ip); diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index c6214cfc575f..3b7915af1f62 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -1120,14 +1120,14 @@ sess_establish_session(struct sess_data *sess_data) struct cifs_ses *ses = sess_data->ses; struct TCP_Server_Info *server = sess_data->server; - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); if (!server->session_estab) { if (server->sign) { server->session_key.response = kmemdup(ses->auth_key.response, ses->auth_key.len, GFP_KERNEL); if (!server->session_key.response) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return -ENOMEM; } server->session_key.len = @@ -1136,7 +1136,7 @@ sess_establish_session(struct sess_data *sess_data) server->sequence_number = 0x2; server->session_estab = true; } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "CIFS session established successfully\n"); return 0; diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c index c71c9a44bef4..2e20ee4dab7b 100644 --- a/fs/cifs/smb1ops.c +++ b/fs/cifs/smb1ops.c @@ -38,10 +38,10 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_rqst *rqst, in_buf->WordCount = 0; put_bcc(0, in_buf); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = cifs_sign_smb(in_buf, server, &mid->sequence_number); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } @@ -55,7 +55,7 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_rqst *rqst, if (rc < 0) server->sequence_number--; - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "issued NT_CANCEL for mid %u, rc = %d\n", get_mid(in_buf), rc); diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 084be3a90198..0e8c85249579 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1369,13 +1369,13 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data) struct cifs_ses *ses = sess_data->ses; struct TCP_Server_Info *server = sess_data->server; - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); if (server->ops->generate_signingkey) { rc = server->ops->generate_signingkey(ses, server); if (rc) { cifs_dbg(FYI, "SMB3 session key generation failed\n"); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } } @@ -1383,7 +1383,7 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data) server->sequence_number = 0x2; server->session_estab = true; } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); cifs_dbg(FYI, "SMB2/3 session established successfully\n"); return rc; diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c index c3278db1cade..5fbbec22bcc8 100644 --- a/fs/cifs/smbdirect.c +++ b/fs/cifs/smbdirect.c @@ -1382,9 +1382,9 @@ void smbd_destroy(struct TCP_Server_Info *server) log_rdma_event(INFO, "freeing mr list\n"); wake_up_interruptible_all(&info->wait_mr); while (atomic_read(&info->mr_used_count)) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); msleep(1000); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); } destroy_mr_list(info); diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 05eca41e3b1e..bfc9bd55870a 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -822,7 +822,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst, } else instance = exist_credits->instance; - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); /* * We can't use credits obtained from the previous session to send this @@ -830,14 +830,14 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst, * return -EAGAIN in such cases to let callers handle it. */ if (instance != server->reconnect_instance) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); add_credits_and_wake_if(server, &credits, optype); return -EAGAIN; } mid = server->ops->setup_async_request(server, rqst); if (IS_ERR(mid)) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); add_credits_and_wake_if(server, &credits, optype); return PTR_ERR(mid); } @@ -868,7 +868,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst, cifs_delete_mid(mid); } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc == 0) return 0; @@ -1109,7 +1109,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, * of smb data. */ - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); /* * All the parts of the compound chain belong obtained credits from the @@ -1119,7 +1119,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, * handle it. */ if (instance != server->reconnect_instance) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); for (j = 0; j < num_rqst; j++) add_credits(server, &credits[j], optype); return -EAGAIN; @@ -1131,7 +1131,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, revert_current_mid(server, i); for (j = 0; j < i; j++) cifs_delete_mid(midQ[j]); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); /* Update # of requests on wire to server */ for (j = 0; j < num_rqst; j++) @@ -1163,7 +1163,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, server->sequence_number -= 2; } - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); /* * If sending failed for some reason or it is an oplock break that we @@ -1190,9 +1190,9 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, if ((ses->ses_status == SES_NEW) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) { spin_unlock(&cifs_tcp_ses_lock); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); smb311_update_preauth_hash(ses, server, rqst[0].rq_iov, rqst[0].rq_nvec); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); spin_lock(&cifs_tcp_ses_lock); } @@ -1266,9 +1266,9 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses, .iov_len = resp_iov[0].iov_len }; spin_unlock(&cifs_tcp_ses_lock); - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); smb311_update_preauth_hash(ses, server, &iov, 1); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); spin_lock(&cifs_tcp_ses_lock); } spin_unlock(&cifs_tcp_ses_lock); @@ -1385,11 +1385,11 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses, and avoid races inside tcp sendmsg code that could cause corruption of smb data */ - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = allocate_mid(ses, in_buf, &midQ); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); /* Update # of requests on wire to server */ add_credits(server, &credits, 0); return rc; @@ -1397,7 +1397,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses, rc = cifs_sign_smb(in_buf, server, &midQ->sequence_number); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); goto out; } @@ -1411,7 +1411,7 @@ SendReceive(const unsigned int xid, struct cifs_ses *ses, if (rc < 0) server->sequence_number -= 2; - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc < 0) goto out; @@ -1530,18 +1530,18 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon, and avoid races inside tcp sendmsg code that could cause corruption of smb data */ - mutex_lock(&server->srv_mutex); + cifs_server_lock(server); rc = allocate_mid(ses, in_buf, &midQ); if (rc) { - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } rc = cifs_sign_smb(in_buf, server, &midQ->sequence_number); if (rc) { cifs_delete_mid(midQ); - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); return rc; } @@ -1554,7 +1554,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifs_tcon *tcon, if (rc < 0) server->sequence_number -= 2; - mutex_unlock(&server->srv_mutex); + cifs_server_unlock(server); if (rc < 0) { cifs_delete_mid(midQ); -- cgit v1.2.3 From 387ba9bf4cb80277fb01a7b753da611ab1260a1a Mon Sep 17 00:00:00 2001 From: Steve French Date: Wed, 1 Jun 2022 21:25:43 -0500 Subject: cifs: do not build smb1ops if legacy support is disabled We should not be including unused SMB1/CIFS functions when legacy support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned off), but especially obvious is not needing to build smb1ops.c at all when legacy support is disabled. Over time we can move more SMB1/CIFS and SMB2.0 legacy functions into ifdefs but this is a good start (and shrinks the module size a few percent). Reviewed-by: Ronnie Sahlberg Signed-off-by: Steve French --- fs/cifs/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/cifs/Makefile b/fs/cifs/Makefile index cc8fdcb35b71..8c9f2c00be72 100644 --- a/fs/cifs/Makefile +++ b/fs/cifs/Makefile @@ -8,7 +8,7 @@ obj-$(CONFIG_CIFS) += cifs.o cifs-y := trace.o cifsfs.o cifssmb.o cifs_debug.o connect.o dir.o file.o \ inode.o link.o misc.o netmisc.o smbencrypt.o transport.o \ cifs_unicode.o nterr.o cifsencrypt.o \ - readdir.o ioctl.o sess.o export.o smb1ops.o unc.o winucase.o \ + readdir.o ioctl.o sess.o export.o unc.o winucase.o \ smb2ops.o smb2maperror.o smb2transport.o \ smb2misc.o smb2pdu.o smb2inode.o smb2file.o cifsacl.o fs_context.o \ dns_resolve.o cifs_spnego_negtokeninit.asn1.o asn1.o @@ -30,3 +30,5 @@ cifs-$(CONFIG_CIFS_FSCACHE) += fscache.o cifs-$(CONFIG_CIFS_SMB_DIRECT) += smbdirect.o cifs-$(CONFIG_CIFS_ROOT) += cifsroot.o + +cifs-$(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) += smb1ops.o -- cgit v1.2.3 From 7ef93ffccd55fb0ba000ed16ef6a81cd7dee07b5 Mon Sep 17 00:00:00 2001 From: Steve French Date: Wed, 1 Jun 2022 22:08:46 -0500 Subject: cifs: version operations for smb20 unneeded when legacy support disabled We should not be including unused smb20 specific code when legacy support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned off). For example smb2_operations and smb2_values aren't used in that case. Over time we can move more and more SMB1/CIFS and SMB2.0 code into the insecure legacy ifdefs Reviewed-by: Ronnie Sahlberg Signed-off-by: Steve French --- fs/cifs/cifsglob.h | 4 +++- fs/cifs/smb2ops.c | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index d589e687611d..f873379066c7 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -1963,11 +1963,13 @@ extern mempool_t *cifs_mid_poolp; /* Operations for different SMB versions */ #define SMB1_VERSION_STRING "1.0" +#define SMB20_VERSION_STRING "2.0" +#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY extern struct smb_version_operations smb1_operations; extern struct smb_version_values smb1_values; -#define SMB20_VERSION_STRING "2.0" extern struct smb_version_operations smb20_operations; extern struct smb_version_values smb20_values; +#endif /* CIFS_ALLOW_INSECURE_LEGACY */ #define SMB21_VERSION_STRING "2.1" extern struct smb_version_operations smb21_operations; extern struct smb_version_values smb21_values; diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index 03ab28c341c4..98a76fa791c0 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -4345,11 +4345,13 @@ smb3_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock, } } +#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY static bool smb2_is_read_op(__u32 oplock) { return oplock == SMB2_OPLOCK_LEVEL_II; } +#endif /* CIFS_ALLOW_INSECURE_LEGACY */ static bool smb21_is_read_op(__u32 oplock) @@ -5448,7 +5450,7 @@ out: return rc; } - +#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY struct smb_version_operations smb20_operations = { .compare_fids = smb2_compare_fids, .setup_request = smb2_setup_request, @@ -5547,6 +5549,7 @@ struct smb_version_operations smb20_operations = { .is_status_io_timeout = smb2_is_status_io_timeout, .is_network_name_deleted = smb2_is_network_name_deleted, }; +#endif /* CIFS_ALLOW_INSECURE_LEGACY */ struct smb_version_operations smb21_operations = { .compare_fids = smb2_compare_fids, @@ -5878,6 +5881,7 @@ struct smb_version_operations smb311_operations = { .is_network_name_deleted = smb2_is_network_name_deleted, }; +#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY struct smb_version_values smb20_values = { .version_string = SMB20_VERSION_STRING, .protocol_id = SMB20_PROT_ID, @@ -5898,6 +5902,7 @@ struct smb_version_values smb20_values = { .signing_required = SMB2_NEGOTIATE_SIGNING_REQUIRED, .create_lease_size = sizeof(struct create_lease), }; +#endif /* ALLOW_INSECURE_LEGACY */ struct smb_version_values smb21_values = { .version_string = SMB21_VERSION_STRING, -- cgit v1.2.3 From 096c956b0d7a4a51b31ffabc1a7398a479d59307 Mon Sep 17 00:00:00 2001 From: Steve French Date: Wed, 1 Jun 2022 23:23:09 -0500 Subject: cifs: update internal module number To 2.37 Signed-off-by: Steve French --- fs/cifs/cifsfs.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h index c0542bdcd06b..dd7e070ca243 100644 --- a/fs/cifs/cifsfs.h +++ b/fs/cifs/cifsfs.h @@ -152,6 +152,7 @@ extern struct dentry *cifs_smb3_do_mount(struct file_system_type *fs_type, extern const struct export_operations cifs_export_ops; #endif /* CONFIG_CIFS_NFSD_EXPORT */ -#define SMB3_PRODUCT_BUILD 35 -#define CIFS_VERSION "2.36" +/* when changing internal version - update following two lines at same time */ +#define SMB3_PRODUCT_BUILD 37 +#define CIFS_VERSION "2.37" #endif /* _CIFSFS_H */ -- cgit v1.2.3 From ef605e86821253d16d47a02ce1f766a461614fef Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Fri, 3 Jun 2022 16:13:02 -0300 Subject: cifs: skip trailing separators of prefix paths During DFS failover, prefix paths may change, so make sure to not leave trailing separators when parsing thew in dfs_cache_get_tgt_share(). The separators of prefix paths are already handled by build_path_from_dentry_optional_prefix(). Consider the following DFS link: //dom/dfs/link: [\srv1\share\dir1, \srv2\share\dir1] Before commit: mount.cifs //dom/dfs/link tree connect to \\srv1\share; prefix_path=dir1 disconnect srv1; failover to srv2 tree connect to \\srv2\share; prefix_path=dir1\ mv foo bar ... SMB2 430 Create Request File: dir1\\foo;GetInfo Request FILE_INFO/SMB2_FILE_ALL_INFO;Close Request SMB2 582 Create Response File: dir1\\foo;GetInfo Response;Close Response SMB2 430 Create Request File: dir1\\bar;GetInfo Request FILE_INFO/SMB2_FILE_ALL_INFO;Close Request SMB2 286 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;GetInfo Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;Close Response, Error: STATUS_OBJECT_NAME_NOT_FOUND SMB2 462 Create Request File: dir1\\foo;SetInfo Request FILE_INFO/SMB2_FILE_RENAME_INFO NewName:dir1\\bar;Close Request SMB2 478 Create Response File: dir1\\foo;SetInfo Response, Error: STATUS_OBJECT_NAME_INVALID;Close Response After commit: mount.cifs //dom/dfs/link tree connect to \\srv1\share; prefix_path=dir1 disconnect srv1; failover to srv2 tree connect to \\srv2\share; prefix_path=dir1 mv foo bar ... SMB2 430 Create Request File: dir1\foo;GetInfo Request FILE_INFO/SMB2_FILE_ALL_INFO;Close Request SMB2 582 Create Response File: dir1\foo;GetInfo Response;Close Response SMB2 430 Create Request File: dir1\bar;GetInfo Request FILE_INFO/SMB2_FILE_ALL_INFO;Close Request SMB2 286 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;GetInfo Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;Close Response, Error: STATUS_OBJECT_NAME_NOT_FOUND SMB2 462 Create Request File: dir1\foo;SetInfo Request FILE_INFO/SMB2_FILE_RENAME_INFO NewName:dir1\bar;Close Request SMB2 478 Create Response File: dir1\foo;SetInfo Response;Close Response Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/cifs/dfs_cache.c | 85 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c index aa7d00b5b3e7..7b978a126268 100644 --- a/fs/cifs/dfs_cache.c +++ b/fs/cifs/dfs_cache.c @@ -1229,6 +1229,30 @@ void dfs_cache_put_refsrv_sessions(const uuid_t *mount_id) kref_put(&mg->refcount, mount_group_release); } +/* Extract share from DFS target and return a pointer to prefix path or NULL */ +static const char *parse_target_share(const char *target, char **share) +{ + const char *s, *seps = "/\\"; + size_t len; + + s = strpbrk(target + 1, seps); + if (!s) + return ERR_PTR(-EINVAL); + + len = strcspn(s + 1, seps); + if (!len) + return ERR_PTR(-EINVAL); + s += len; + + len = s - target + 1; + *share = kstrndup(target, len, GFP_KERNEL); + if (!*share) + return ERR_PTR(-ENOMEM); + + s = target + len; + return s + strspn(s, seps); +} + /** * dfs_cache_get_tgt_share - parse a DFS target * @@ -1242,56 +1266,45 @@ void dfs_cache_put_refsrv_sessions(const uuid_t *mount_id) int dfs_cache_get_tgt_share(char *path, const struct dfs_cache_tgt_iterator *it, char **share, char **prefix) { - char *s, sep, *p; - size_t len; - size_t plen1, plen2; + char sep; + char *target_share, *ppath; + const char *target_ppath, *dfsref_ppath; + size_t target_pplen, dfsref_pplen; + size_t len, c; if (!it || !path || !share || !prefix || strlen(path) < it->it_path_consumed) return -EINVAL; - *share = NULL; - *prefix = NULL; - sep = it->it_name[0]; if (sep != '\\' && sep != '/') return -EINVAL; - s = strchr(it->it_name + 1, sep); - if (!s) - return -EINVAL; + target_ppath = parse_target_share(it->it_name, &target_share); + if (IS_ERR(target_ppath)) + return PTR_ERR(target_ppath); - /* point to prefix in target node */ - s = strchrnul(s + 1, sep); + /* point to prefix in DFS referral path */ + dfsref_ppath = path + it->it_path_consumed; + dfsref_ppath += strspn(dfsref_ppath, "/\\"); - /* extract target share */ - *share = kstrndup(it->it_name, s - it->it_name, GFP_KERNEL); - if (!*share) - return -ENOMEM; + target_pplen = strlen(target_ppath); + dfsref_pplen = strlen(dfsref_ppath); - /* skip separator */ - if (*s) - s++; - /* point to prefix in DFS path */ - p = path + it->it_path_consumed; - if (*p == sep) - p++; - - /* merge prefix paths from DFS path and target node */ - plen1 = it->it_name + strlen(it->it_name) - s; - plen2 = path + strlen(path) - p; - if (plen1 || plen2) { - len = plen1 + plen2 + 2; - *prefix = kmalloc(len, GFP_KERNEL); - if (!*prefix) { - kfree(*share); - *share = NULL; + /* merge prefix paths from DFS referral path and target node */ + if (target_pplen || dfsref_pplen) { + len = target_pplen + dfsref_pplen + 2; + ppath = kzalloc(len, GFP_KERNEL); + if (!ppath) { + kfree(target_share); return -ENOMEM; } - if (plen1) - scnprintf(*prefix, len, "%.*s%c%.*s", (int)plen1, s, sep, (int)plen2, p); - else - strscpy(*prefix, p, len); + c = strscpy(ppath, target_ppath, len); + if (c && dfsref_pplen) + ppath[c] = sep; + strlcat(ppath, dfsref_ppath, len); } + *share = target_share; + *prefix = ppath; return 0; } -- cgit v1.2.3 From ee3c8019cce254f586b7fc2c5b836c275b275527 Mon Sep 17 00:00:00 2001 From: Steve French Date: Sat, 4 Jun 2022 01:18:37 -0500 Subject: cifs: fix uninitialized pointer in error case in dfs_cache_get_tgt_share Set default value of ppath to null. Reported-by: kernel test robot Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/cifs/dfs_cache.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c index 7b978a126268..34a8f3baed5e 100644 --- a/fs/cifs/dfs_cache.c +++ b/fs/cifs/dfs_cache.c @@ -1267,7 +1267,8 @@ int dfs_cache_get_tgt_share(char *path, const struct dfs_cache_tgt_iterator *it, char **prefix) { char sep; - char *target_share, *ppath; + char *target_share; + char *ppath = NULL; const char *target_ppath, *dfsref_ppath; size_t target_pplen, dfsref_pplen; size_t len, c; -- cgit v1.2.3