* [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.