From 50bd7d5a647bdf533575111c5335f49707c2ce2f Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Tue, 5 Jul 2022 11:16:24 +0000 Subject: cifs: fix race condition with delayed threads On failure to create a new channel, first cancel the delayed threads, which could try to search for this channel, and not find it. The other option was to put the tcp session for the channel first, before decrementing chan_count. But that would leave a reference to the tcp session, when it has been freed already. So going with the former option and cancelling the delayed works first, before rolling back the channel. Fixes: aa45dadd34e4 ("cifs: change iface_list from array to sorted linked list") Signed-off-by: Shyam Prasad N Acked-by: Enzo Matsumiya Signed-off-by: Steve French --- fs/cifs/sess.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index b85718f32b53..02c8b2906196 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -474,6 +474,14 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses, out: if (rc && chan->server) { + /* + * we should avoid race with these delayed works before we + * remove this channel + */ + cancel_delayed_work_sync(&chan->server->echo); + cancel_delayed_work_sync(&chan->server->resolve); + cancel_delayed_work_sync(&chan->server->reconnect); + spin_lock(&ses->chan_lock); /* we rely on all bits beyond chan_count to be clear */ cifs_chan_clear_need_reconnect(ses, chan->server); @@ -484,10 +492,9 @@ out: */ WARN_ON(ses->chan_count < 1); spin_unlock(&ses->chan_lock); - } - if (rc && chan->server) cifs_put_tcp_session(chan->server, 0); + } return rc; } -- cgit v1.2.3 From 2883f4b5a0e1ac9472c7e3ae9bbdb4ea5a2117c6 Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Wed, 6 Jul 2022 14:29:12 +0000 Subject: cifs: remove unnecessary locking of chan_lock while freeing session In cifs_put_smb_ses, when we're freeing the last ref count to the session, we need to free up each channel. At this point, it is unnecessary to take chan_lock, since we have the last reference to the ses. Picking up this lock also introduced a deadlock because it calls cifs_put_tcp_ses, which locks cifs_tcp_ses_lock. Signed-off-by: Shyam Prasad N Acked-by: Enzo Matsumiya Signed-off-by: Steve French --- fs/cifs/connect.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index fa29c9aae24b..386bb523c69e 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1918,7 +1918,6 @@ void cifs_put_smb_ses(struct cifs_ses *ses) list_del_init(&ses->smb_ses_list); spin_unlock(&cifs_tcp_ses_lock); - spin_lock(&ses->chan_lock); chan_count = ses->chan_count; /* close any extra channels */ @@ -1934,7 +1933,6 @@ void cifs_put_smb_ses(struct cifs_ses *ses) ses->chans[i].server = NULL; } } - spin_unlock(&ses->chan_lock); sesInfoFree(ses); cifs_put_tcp_session(server, 0); -- cgit v1.2.3 From 32f319183c439b239294cb2d70ada3564c4c7c39 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 12 Jul 2022 00:11:42 -0500 Subject: smb3: workaround negprot bug in some Samba servers Mount can now fail to older Samba servers due to a server bug handling padding at the end of the last negotiate context (negotiate contexts typically are rounded up to 8 bytes by adding padding if needed). This server bug can be avoided by switching the order of negotiate contexts, placing a negotiate context at the end that does not require padding (prior to the recent netname context fix this was the case on the client). Fixes: 73130a7b1ac9 ("smb3: fix empty netname context on secondary channels") Reported-by: Julian Sikorski Tested-by: Julian Sikorski Reviewed-by: Shyam Prasad N Signed-off-by: Steve French --- fs/cifs/smb2pdu.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 12b4dddaedb0..c705de32e225 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -571,10 +571,6 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, *total_len += ctxt_len; pneg_ctxt += ctxt_len; - build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt); - *total_len += sizeof(struct smb2_posix_neg_context); - pneg_ctxt += sizeof(struct smb2_posix_neg_context); - /* * secondary channels don't have the hostname field populated * use the hostname field in the primary channel instead @@ -586,9 +582,14 @@ assemble_neg_contexts(struct smb2_negotiate_req *req, hostname); *total_len += ctxt_len; pneg_ctxt += ctxt_len; - neg_context_count = 4; - } else /* second channels do not have a hostname */ neg_context_count = 3; + } else + neg_context_count = 2; + + build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt); + *total_len += sizeof(struct smb2_posix_neg_context); + pneg_ctxt += sizeof(struct smb2_posix_neg_context); + neg_context_count++; if (server->compress_algorithm) { build_compression_ctxt((struct smb2_compression_capabilities_context *) -- cgit v1.2.3