All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath
@ 2023-03-29 20:14 Paulo Alcantara
  2023-03-29 20:14 ` [PATCH 2/5] cifs: get rid of unnecessary ifdef Paulo Alcantara
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Paulo Alcantara @ 2023-03-29 20:14 UTC (permalink / raw)
  To: smfrench; +Cc: linux-cifs, Paulo Alcantara

leaf_fullpath is now part of smb3_fs_context structure, while
origin_fullpath is just a variable in __dfs_mount_share() that is used
when final tcon is available.

Get rid of those fields in cifs_mount_ctx structure and no-op
kfree()'s.

Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
---
 fs/cifs/cifsglob.h | 1 -
 fs/cifs/connect.c  | 2 --
 2 files changed, 3 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 08a73dcb7786..c5e3a0fc7983 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1750,7 +1750,6 @@ struct cifs_mount_ctx {
 	struct TCP_Server_Info *server;
 	struct cifs_ses *ses;
 	struct cifs_tcon *tcon;
-	char *origin_fullpath, *leaf_fullpath;
 	struct list_head dfs_ses_list;
 };
 
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 1cbb90587995..ef50f603e640 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3454,8 +3454,6 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
 
 error:
 	dfs_put_root_smb_sessions(&mnt_ctx.dfs_ses_list);
-	kfree(mnt_ctx.origin_fullpath);
-	kfree(mnt_ctx.leaf_fullpath);
 	cifs_mount_put_conns(&mnt_ctx);
 	return rc;
 }
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] cifs: get rid of unnecessary ifdef
  2023-03-29 20:14 [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath Paulo Alcantara
@ 2023-03-29 20:14 ` Paulo Alcantara
  2023-03-29 20:25   ` ronnie sahlberg
  2023-03-29 20:14 ` [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1 Paulo Alcantara
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Paulo Alcantara @ 2023-03-29 20:14 UTC (permalink / raw)
  To: smfrench; +Cc: linux-cifs, Paulo Alcantara

No need to ifdef around TCP_Server_Info::{origin,leaf}_fullpath as
they're declared regardless CONFIG_CIFS_DFS_UPCALL.

Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
---
 fs/cifs/connect.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ef50f603e640..7c3e090a0f13 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -993,10 +993,8 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
 		 */
 	}
 
-#ifdef CONFIG_CIFS_DFS_UPCALL
 	kfree(server->origin_fullpath);
 	kfree(server->leaf_fullpath);
-#endif
 	kfree(server);
 
 	length = atomic_dec_return(&tcpSesAllocCount);
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1
  2023-03-29 20:14 [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath Paulo Alcantara
  2023-03-29 20:14 ` [PATCH 2/5] cifs: get rid of unnecessary ifdef Paulo Alcantara
@ 2023-03-29 20:14 ` Paulo Alcantara
  2023-03-29 20:27   ` ronnie sahlberg
  2023-03-30 22:59   ` Steve French
  2023-03-29 20:14 ` [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer() Paulo Alcantara
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Paulo Alcantara @ 2023-03-29 20:14 UTC (permalink / raw)
  To: smfrench; +Cc: linux-cifs, Paulo Alcantara

Prevent multiple threads of doing negotiate, session setup and tree
connect by holding @ses->session_mutex in cifs_reconnect_tcon() while
reconnecting session and tcon.

Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
---
 fs/cifs/cifssmb.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 38a697eca305..c9d57ba84be4 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -71,7 +71,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	int rc;
 	struct cifs_ses *ses;
 	struct TCP_Server_Info *server;
-	struct nls_table *nls_codepage;
+	struct nls_table *nls_codepage = NULL;
 
 	/*
 	 * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for
@@ -99,6 +99,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	}
 	spin_unlock(&tcon->tc_lock);
 
+again:
 	rc = cifs_wait_for_server_reconnect(server, tcon->retry);
 	if (rc)
 		return rc;
@@ -110,8 +111,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	}
 	spin_unlock(&ses->chan_lock);
 
-	nls_codepage = load_nls_default();
-
+	mutex_lock(&ses->session_mutex);
 	/*
 	 * Recheck after acquire mutex. If another thread is negotiating
 	 * and the server never sends an answer the socket will be closed
@@ -120,29 +120,38 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	spin_lock(&server->srv_lock);
 	if (server->tcpStatus == CifsNeedReconnect) {
 		spin_unlock(&server->srv_lock);
+		mutex_lock(&ses->session_mutex);
+
+		if (tcon->retry)
+			goto again;
 		rc = -EHOSTDOWN;
 		goto out;
 	}
 	spin_unlock(&server->srv_lock);
 
+	nls_codepage = load_nls_default();
+
 	/*
 	 * need to prevent multiple threads trying to simultaneously
 	 * reconnect the same SMB session
 	 */
+	spin_lock(&ses->ses_lock);
 	spin_lock(&ses->chan_lock);
-	if (!cifs_chan_needs_reconnect(ses, server)) {
+	if (!cifs_chan_needs_reconnect(ses, server) &&
+	    ses->ses_status == SES_GOOD) {
 		spin_unlock(&ses->chan_lock);
+		spin_unlock(&ses->ses_lock);
 
 		/* this means that we only need to tree connect */
 		if (tcon->need_reconnect)
 			goto skip_sess_setup;
 
-		rc = -EHOSTDOWN;
+		mutex_unlock(&ses->session_mutex);
 		goto out;
 	}
 	spin_unlock(&ses->chan_lock);
+	spin_unlock(&ses->ses_lock);
 
-	mutex_lock(&ses->session_mutex);
 	rc = cifs_negotiate_protocol(0, ses, server);
 	if (!rc)
 		rc = cifs_setup_session(0, ses, server, nls_codepage);
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer()
  2023-03-29 20:14 [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath Paulo Alcantara
  2023-03-29 20:14 ` [PATCH 2/5] cifs: get rid of unnecessary ifdef Paulo Alcantara
  2023-03-29 20:14 ` [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1 Paulo Alcantara
@ 2023-03-29 20:14 ` Paulo Alcantara
  2023-03-29 20:27   ` ronnie sahlberg
  2023-03-30 22:59   ` Steve French
  2023-03-29 20:14 ` [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect() Paulo Alcantara
  2023-03-29 20:24 ` [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath ronnie sahlberg
  4 siblings, 2 replies; 13+ messages in thread
From: Paulo Alcantara @ 2023-03-29 20:14 UTC (permalink / raw)
  To: smfrench; +Cc: linux-cifs, Paulo Alcantara

We can't call smb_init() in CIFSGetDFSRefer() as cifs_reconnect_tcon()
may end up calling CIFSGetDFSRefer() again to get new DFS referrals
and thus causing an infinite recursion.

Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
---
 fs/cifs/cifssmb.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index c9d57ba84be4..0d30b17494e4 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4382,8 +4382,13 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 		return -ENODEV;
 
 getDFSRetry:
-	rc = smb_init(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc, (void **) &pSMB,
-		      (void **) &pSMBr);
+	/*
+	 * Use smb_init_no_reconnect() instead of smb_init() as
+	 * CIFSGetDFSRefer() may be called from cifs_reconnect_tcon() and thus
+	 * causing an infinite recursion.
+	 */
+	rc = smb_init_no_reconnect(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc,
+				   (void **)&pSMB, (void **)&pSMBr);
 	if (rc)
 		return rc;
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect()
  2023-03-29 20:14 [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath Paulo Alcantara
                   ` (2 preceding siblings ...)
  2023-03-29 20:14 ` [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer() Paulo Alcantara
@ 2023-03-29 20:14 ` Paulo Alcantara
  2023-03-29 20:28   ` ronnie sahlberg
  2023-03-30 22:59   ` Steve French
  2023-03-29 20:24 ` [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath ronnie sahlberg
  4 siblings, 2 replies; 13+ messages in thread
From: Paulo Alcantara @ 2023-03-29 20:14 UTC (permalink / raw)
  To: smfrench; +Cc: linux-cifs, Paulo Alcantara

The SMB2_IOCTL check in the switch statement will never be true as we
return earlier from smb2_reconnect() if @smb2_command == SMB2_IOCTL.

Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
---
 fs/cifs/smb2pdu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6bd2aa6af18f..2b92132097dc 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -310,7 +310,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
 	case SMB2_READ:
 	case SMB2_WRITE:
 	case SMB2_LOCK:
-	case SMB2_IOCTL:
 	case SMB2_QUERY_DIRECTORY:
 	case SMB2_CHANGE_NOTIFY:
 	case SMB2_QUERY_INFO:
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath
  2023-03-29 20:14 [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath Paulo Alcantara
                   ` (3 preceding siblings ...)
  2023-03-29 20:14 ` [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect() Paulo Alcantara
@ 2023-03-29 20:24 ` ronnie sahlberg
  4 siblings, 0 replies; 13+ messages in thread
From: ronnie sahlberg @ 2023-03-29 20:24 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: smfrench, linux-cifs

Reviewed-by me

On Thu, 30 Mar 2023 at 06:20, Paulo Alcantara <pc@manguebit.com> wrote:
>
> leaf_fullpath is now part of smb3_fs_context structure, while
> origin_fullpath is just a variable in __dfs_mount_share() that is used
> when final tcon is available.
>
> Get rid of those fields in cifs_mount_ctx structure and no-op
> kfree()'s.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/cifsglob.h | 1 -
>  fs/cifs/connect.c  | 2 --
>  2 files changed, 3 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 08a73dcb7786..c5e3a0fc7983 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1750,7 +1750,6 @@ struct cifs_mount_ctx {
>         struct TCP_Server_Info *server;
>         struct cifs_ses *ses;
>         struct cifs_tcon *tcon;
> -       char *origin_fullpath, *leaf_fullpath;
>         struct list_head dfs_ses_list;
>  };
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 1cbb90587995..ef50f603e640 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3454,8 +3454,6 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
>
>  error:
>         dfs_put_root_smb_sessions(&mnt_ctx.dfs_ses_list);
> -       kfree(mnt_ctx.origin_fullpath);
> -       kfree(mnt_ctx.leaf_fullpath);
>         cifs_mount_put_conns(&mnt_ctx);
>         return rc;
>  }
> --
> 2.40.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] cifs: get rid of unnecessary ifdef
  2023-03-29 20:14 ` [PATCH 2/5] cifs: get rid of unnecessary ifdef Paulo Alcantara
@ 2023-03-29 20:25   ` ronnie sahlberg
  0 siblings, 0 replies; 13+ messages in thread
From: ronnie sahlberg @ 2023-03-29 20:25 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: smfrench, linux-cifs

reviewed-by me

On Thu, 30 Mar 2023 at 06:20, Paulo Alcantara <pc@manguebit.com> wrote:
>
> No need to ifdef around TCP_Server_Info::{origin,leaf}_fullpath as
> they're declared regardless CONFIG_CIFS_DFS_UPCALL.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/connect.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ef50f603e640..7c3e090a0f13 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -993,10 +993,8 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
>                  */
>         }
>
> -#ifdef CONFIG_CIFS_DFS_UPCALL
>         kfree(server->origin_fullpath);
>         kfree(server->leaf_fullpath);
> -#endif
>         kfree(server);
>
>         length = atomic_dec_return(&tcpSesAllocCount);
> --
> 2.40.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1
  2023-03-29 20:14 ` [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1 Paulo Alcantara
@ 2023-03-29 20:27   ` ronnie sahlberg
  2023-03-30 22:59   ` Steve French
  1 sibling, 0 replies; 13+ messages in thread
From: ronnie sahlberg @ 2023-03-29 20:27 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: smfrench, linux-cifs

reviewed-by me

On Thu, 30 Mar 2023 at 06:20, Paulo Alcantara <pc@manguebit.com> wrote:
>
> Prevent multiple threads of doing negotiate, session setup and tree
> connect by holding @ses->session_mutex in cifs_reconnect_tcon() while
> reconnecting session and tcon.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/cifssmb.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 38a697eca305..c9d57ba84be4 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -71,7 +71,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         int rc;
>         struct cifs_ses *ses;
>         struct TCP_Server_Info *server;
> -       struct nls_table *nls_codepage;
> +       struct nls_table *nls_codepage = NULL;
>
>         /*
>          * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for
> @@ -99,6 +99,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         }
>         spin_unlock(&tcon->tc_lock);
>
> +again:
>         rc = cifs_wait_for_server_reconnect(server, tcon->retry);
>         if (rc)
>                 return rc;
> @@ -110,8 +111,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         }
>         spin_unlock(&ses->chan_lock);
>
> -       nls_codepage = load_nls_default();
> -
> +       mutex_lock(&ses->session_mutex);
>         /*
>          * Recheck after acquire mutex. If another thread is negotiating
>          * and the server never sends an answer the socket will be closed
> @@ -120,29 +120,38 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         spin_lock(&server->srv_lock);
>         if (server->tcpStatus == CifsNeedReconnect) {
>                 spin_unlock(&server->srv_lock);
> +               mutex_lock(&ses->session_mutex);
> +
> +               if (tcon->retry)
> +                       goto again;
>                 rc = -EHOSTDOWN;
>                 goto out;
>         }
>         spin_unlock(&server->srv_lock);
>
> +       nls_codepage = load_nls_default();
> +
>         /*
>          * need to prevent multiple threads trying to simultaneously
>          * reconnect the same SMB session
>          */
> +       spin_lock(&ses->ses_lock);
>         spin_lock(&ses->chan_lock);
> -       if (!cifs_chan_needs_reconnect(ses, server)) {
> +       if (!cifs_chan_needs_reconnect(ses, server) &&
> +           ses->ses_status == SES_GOOD) {
>                 spin_unlock(&ses->chan_lock);
> +               spin_unlock(&ses->ses_lock);
>
>                 /* this means that we only need to tree connect */
>                 if (tcon->need_reconnect)
>                         goto skip_sess_setup;
>
> -               rc = -EHOSTDOWN;
> +               mutex_unlock(&ses->session_mutex);
>                 goto out;
>         }
>         spin_unlock(&ses->chan_lock);
> +       spin_unlock(&ses->ses_lock);
>
> -       mutex_lock(&ses->session_mutex);
>         rc = cifs_negotiate_protocol(0, ses, server);
>         if (!rc)
>                 rc = cifs_setup_session(0, ses, server, nls_codepage);
> --
> 2.40.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer()
  2023-03-29 20:14 ` [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer() Paulo Alcantara
@ 2023-03-29 20:27   ` ronnie sahlberg
  2023-03-30 22:59   ` Steve French
  1 sibling, 0 replies; 13+ messages in thread
From: ronnie sahlberg @ 2023-03-29 20:27 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: smfrench, linux-cifs

reviewed-by me

On Thu, 30 Mar 2023 at 06:20, Paulo Alcantara <pc@manguebit.com> wrote:
>
> We can't call smb_init() in CIFSGetDFSRefer() as cifs_reconnect_tcon()
> may end up calling CIFSGetDFSRefer() again to get new DFS referrals
> and thus causing an infinite recursion.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/cifssmb.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index c9d57ba84be4..0d30b17494e4 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -4382,8 +4382,13 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
>                 return -ENODEV;
>
>  getDFSRetry:
> -       rc = smb_init(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc, (void **) &pSMB,
> -                     (void **) &pSMBr);
> +       /*
> +        * Use smb_init_no_reconnect() instead of smb_init() as
> +        * CIFSGetDFSRefer() may be called from cifs_reconnect_tcon() and thus
> +        * causing an infinite recursion.
> +        */
> +       rc = smb_init_no_reconnect(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc,
> +                                  (void **)&pSMB, (void **)&pSMBr);
>         if (rc)
>                 return rc;
>
> --
> 2.40.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect()
  2023-03-29 20:14 ` [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect() Paulo Alcantara
@ 2023-03-29 20:28   ` ronnie sahlberg
  2023-03-30 22:59   ` Steve French
  1 sibling, 0 replies; 13+ messages in thread
From: ronnie sahlberg @ 2023-03-29 20:28 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: smfrench, linux-cifs

reviewed-by me

On Thu, 30 Mar 2023 at 06:20, Paulo Alcantara <pc@manguebit.com> wrote:
>
> The SMB2_IOCTL check in the switch statement will never be true as we
> return earlier from smb2_reconnect() if @smb2_command == SMB2_IOCTL.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/smb2pdu.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 6bd2aa6af18f..2b92132097dc 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -310,7 +310,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
>         case SMB2_READ:
>         case SMB2_WRITE:
>         case SMB2_LOCK:
> -       case SMB2_IOCTL:
>         case SMB2_QUERY_DIRECTORY:
>         case SMB2_CHANGE_NOTIFY:
>         case SMB2_QUERY_INFO:
> --
> 2.40.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1
  2023-03-29 20:14 ` [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1 Paulo Alcantara
  2023-03-29 20:27   ` ronnie sahlberg
@ 2023-03-30 22:59   ` Steve French
  1 sibling, 0 replies; 13+ messages in thread
From: Steve French @ 2023-03-30 22:59 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: linux-cifs

added ronnie's RB and merged into cifs-2.6.git for-next (I will hold
off on patches 1 and 2 since they are cleanup until 6.4-rc)

On Wed, Mar 29, 2023 at 3:14 PM Paulo Alcantara <pc@manguebit.com> wrote:
>
> Prevent multiple threads of doing negotiate, session setup and tree
> connect by holding @ses->session_mutex in cifs_reconnect_tcon() while
> reconnecting session and tcon.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/cifssmb.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 38a697eca305..c9d57ba84be4 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -71,7 +71,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         int rc;
>         struct cifs_ses *ses;
>         struct TCP_Server_Info *server;
> -       struct nls_table *nls_codepage;
> +       struct nls_table *nls_codepage = NULL;
>
>         /*
>          * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for
> @@ -99,6 +99,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         }
>         spin_unlock(&tcon->tc_lock);
>
> +again:
>         rc = cifs_wait_for_server_reconnect(server, tcon->retry);
>         if (rc)
>                 return rc;
> @@ -110,8 +111,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         }
>         spin_unlock(&ses->chan_lock);
>
> -       nls_codepage = load_nls_default();
> -
> +       mutex_lock(&ses->session_mutex);
>         /*
>          * Recheck after acquire mutex. If another thread is negotiating
>          * and the server never sends an answer the socket will be closed
> @@ -120,29 +120,38 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>         spin_lock(&server->srv_lock);
>         if (server->tcpStatus == CifsNeedReconnect) {
>                 spin_unlock(&server->srv_lock);
> +               mutex_lock(&ses->session_mutex);
> +
> +               if (tcon->retry)
> +                       goto again;
>                 rc = -EHOSTDOWN;
>                 goto out;
>         }
>         spin_unlock(&server->srv_lock);
>
> +       nls_codepage = load_nls_default();
> +
>         /*
>          * need to prevent multiple threads trying to simultaneously
>          * reconnect the same SMB session
>          */
> +       spin_lock(&ses->ses_lock);
>         spin_lock(&ses->chan_lock);
> -       if (!cifs_chan_needs_reconnect(ses, server)) {
> +       if (!cifs_chan_needs_reconnect(ses, server) &&
> +           ses->ses_status == SES_GOOD) {
>                 spin_unlock(&ses->chan_lock);
> +               spin_unlock(&ses->ses_lock);
>
>                 /* this means that we only need to tree connect */
>                 if (tcon->need_reconnect)
>                         goto skip_sess_setup;
>
> -               rc = -EHOSTDOWN;
> +               mutex_unlock(&ses->session_mutex);
>                 goto out;
>         }
>         spin_unlock(&ses->chan_lock);
> +       spin_unlock(&ses->ses_lock);
>
> -       mutex_lock(&ses->session_mutex);
>         rc = cifs_negotiate_protocol(0, ses, server);
>         if (!rc)
>                 rc = cifs_setup_session(0, ses, server, nls_codepage);
> --
> 2.40.0
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer()
  2023-03-29 20:14 ` [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer() Paulo Alcantara
  2023-03-29 20:27   ` ronnie sahlberg
@ 2023-03-30 22:59   ` Steve French
  1 sibling, 0 replies; 13+ messages in thread
From: Steve French @ 2023-03-30 22:59 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: linux-cifs

added cc:stable and added ronnie's RB and merged into cifs-2.6.git for-next

On Wed, Mar 29, 2023 at 3:14 PM Paulo Alcantara <pc@manguebit.com> wrote:
>
> We can't call smb_init() in CIFSGetDFSRefer() as cifs_reconnect_tcon()
> may end up calling CIFSGetDFSRefer() again to get new DFS referrals
> and thus causing an infinite recursion.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/cifssmb.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index c9d57ba84be4..0d30b17494e4 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -4382,8 +4382,13 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
>                 return -ENODEV;
>
>  getDFSRetry:
> -       rc = smb_init(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc, (void **) &pSMB,
> -                     (void **) &pSMBr);
> +       /*
> +        * Use smb_init_no_reconnect() instead of smb_init() as
> +        * CIFSGetDFSRefer() may be called from cifs_reconnect_tcon() and thus
> +        * causing an infinite recursion.
> +        */
> +       rc = smb_init_no_reconnect(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc,
> +                                  (void **)&pSMB, (void **)&pSMBr);
>         if (rc)
>                 return rc;
>
> --
> 2.40.0
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect()
  2023-03-29 20:14 ` [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect() Paulo Alcantara
  2023-03-29 20:28   ` ronnie sahlberg
@ 2023-03-30 22:59   ` Steve French
  1 sibling, 0 replies; 13+ messages in thread
From: Steve French @ 2023-03-30 22:59 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: linux-cifs

added ronnie's RB and merged into cifs-2.6.git for-next

On Wed, Mar 29, 2023 at 3:14 PM Paulo Alcantara <pc@manguebit.com> wrote:
>
> The SMB2_IOCTL check in the switch statement will never be true as we
> return earlier from smb2_reconnect() if @smb2_command == SMB2_IOCTL.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
>  fs/cifs/smb2pdu.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 6bd2aa6af18f..2b92132097dc 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -310,7 +310,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
>         case SMB2_READ:
>         case SMB2_WRITE:
>         case SMB2_LOCK:
> -       case SMB2_IOCTL:
>         case SMB2_QUERY_DIRECTORY:
>         case SMB2_CHANGE_NOTIFY:
>         case SMB2_QUERY_INFO:
> --
> 2.40.0
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-03-30 22:59 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-29 20:14 [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath Paulo Alcantara
2023-03-29 20:14 ` [PATCH 2/5] cifs: get rid of unnecessary ifdef Paulo Alcantara
2023-03-29 20:25   ` ronnie sahlberg
2023-03-29 20:14 ` [PATCH 3/5] cifs: avoid races in parallel reconnects in smb1 Paulo Alcantara
2023-03-29 20:27   ` ronnie sahlberg
2023-03-30 22:59   ` Steve French
2023-03-29 20:14 ` [PATCH 4/5] cifs: prevent infinite recursion in CIFSGetDFSRefer() Paulo Alcantara
2023-03-29 20:27   ` ronnie sahlberg
2023-03-30 22:59   ` Steve French
2023-03-29 20:14 ` [PATCH 5/5] cifs: get rid of dead check in smb2_reconnect() Paulo Alcantara
2023-03-29 20:28   ` ronnie sahlberg
2023-03-30 22:59   ` Steve French
2023-03-29 20:24 ` [PATCH 1/5] cifs: get rid of cifs_mount_ctx::{origin,leaf}_fullpath ronnie sahlberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.