linux-next: Fixes tag needs some work in the cifs tree

linux-next: Fixes tag needs some work in the cifs tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

Hi all,

In commit

  37c1ad516f2c ("cifs: fix race condition with delayed threads")

Fixes tag

  Fixes: 37d488b3d38c ("cifs: change iface_list from array to sorted linked list")

has these problem(s):

  - Target SHA1 does not exist

Maybe you meant

Fixes: aa45dadd34e4 ("cifs: change iface_list from array to sorted linked list")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [EXTERNAL] linux-next: Fixes tag needs some work in the cifs tree

[Shyam Prasad]

-----Original Message-----
From: Stephen Rothwell <sfr@canb.auug.org.au> 
Sent: Wednesday, July 6, 2022 3:36 AM
To: Steve French <smfrench@gmail.com>; CIFS <linux-cifs@vger.kernel.org>
Cc: Shyam Prasad <Shyam.Prasad@microsoft.com>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Linux Next Mailing List <linux-next@vger.kernel.org>
Subject: [EXTERNAL] linux-next: Fixes tag needs some work in the cifs tree

Hi all,

In commit

  37c1ad516f2c ("cifs: fix race condition with delayed threads")

Fixes tag

  Fixes: 37d488b3d38c ("cifs: change iface_list from array to sorted linked list")

has these problem(s):

  - Target SHA1 does not exist

Maybe you meant

Fixes: aa45dadd34e4 ("cifs: change iface_list from array to sorted linked list")

-- 
Cheers,
Stephen Rothwell

-------------------------------------

Good catch. I must have used a different branch.
Steve, can you please fix the SHA1 in your branch?

Regards,
Shyam

Re: [EXTERNAL] linux-next: Fixes tag needs some work in the cifs tree

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

Fixed

(updated patch also attached)

On Wed, Jul 6, 2022 at 12:37 AM Shyam Prasad <Shyam.Prasad@microsoft.com> wrote:
>
> -----Original Message-----
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Sent: Wednesday, July 6, 2022 3:36 AM
> To: Steve French <smfrench@gmail.com>; CIFS <linux-cifs@vger.kernel.org>
> Cc: Shyam Prasad <Shyam.Prasad@microsoft.com>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Linux Next Mailing List <linux-next@vger.kernel.org>
> Subject: [EXTERNAL] linux-next: Fixes tag needs some work in the cifs tree
>
> Hi all,
>
> In commit
>
>   37c1ad516f2c ("cifs: fix race condition with delayed threads")
>
> Fixes tag
>
>   Fixes: 37d488b3d38c ("cifs: change iface_list from array to sorted linked list")
>
> has these problem(s):
>
>   - Target SHA1 does not exist
>
> Maybe you meant
>
> Fixes: aa45dadd34e4 ("cifs: change iface_list from array to sorted linked list")
>
> --
> Cheers,
> Stephen Rothwell
>
> -------------------------------------
>
> Good catch. I must have used a different branch.
> Steve, can you please fix the SHA1 in your branch?
>
> Regards,
> Shyam



-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-fix-race-condition-with-delayed-threads.patch --]
[-- Type: text/x-patch, Size: 1856 bytes --]

From ac125d93ae92d9a55db35c56e860658b53208f01 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Tue, 5 Jul 2022 11:16:24 +0000
Subject: [PATCH 1/2] 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 <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/sess.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

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 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
 		 */
 		WARN_ON(ses->chan_count < 1);
 		spin_unlock(&ses->chan_lock);
-	}
 
-	if (rc && chan->server)
 		cifs_put_tcp_session(chan->server, 0);
+	}
 
 	return rc;
 }
-- 
2.34.1