diff mbox series

cifs: serialize all mount attempts

Message ID 20220118074512.2153136-1-lsahlber@redhat.com (mailing list archive)
State New, archived
Headers show
Series cifs: serialize all mount attempts | expand

Commit Message

Ronnie Sahlberg Jan. 18, 2022, 7:45 a.m. UTC
RHBZ: 2008434

If we try to perform multiple concurrent mounts ot the same server we might
end up in a situation where:
Thread #1                          Thread #2
    creates TCP connection
    Issues NegotiateProtocol
    ...                            Pick the TCP connection for Thread #1
                                   Issue a new NegotiateProtocol

which then leads to the the server kills off the session.
There are also other a similar race where several threads ending up
withe their own unique tcp connection that all go to the same server structure ....

The most straightforward way to fix these races with concurrent mounts are to serialize
them. I.e. only allow one mount to be in progress at a time.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/fs_context.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Steve French Jan. 18, 2022, 4:23 p.m. UTC | #1
Am curious why serializing on a per-socket mutex
(TCP_Server_info)->srv_mutex doesn't work?

On Tue, Jan 18, 2022 at 1:45 AM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ: 2008434
>
> If we try to perform multiple concurrent mounts ot the same server we might
> end up in a situation where:
> Thread #1                          Thread #2
>     creates TCP connection
>     Issues NegotiateProtocol
>     ...                            Pick the TCP connection for Thread #1
>                                    Issue a new NegotiateProtocol
>
> which then leads to the the server kills off the session.
> There are also other a similar race where several threads ending up
> withe their own unique tcp connection that all go to the same server structure ....
>
> The most straightforward way to fix these races with concurrent mounts are to serialize
> them. I.e. only allow one mount to be in progress at a time.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/fs_context.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index e3ed25dc6f3f..7ec35f3f0a5f 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -37,6 +37,8 @@
>  #include "rfc1002pdu.h"
>  #include "fs_context.h"
>
> +static DEFINE_MUTEX(cifs_mount_mutex);
> +
>  static const match_table_t cifs_smb_version_tokens = {
>         { Smb_1, SMB1_VERSION_STRING },
>         { Smb_20, SMB20_VERSION_STRING},
> @@ -707,10 +709,14 @@ static int smb3_get_tree_common(struct fs_context *fc)
>  static int smb3_get_tree(struct fs_context *fc)
>  {
>         int err = smb3_fs_context_validate(fc);
> +       int ret;
>
>         if (err)
>                 return err;
> -       return smb3_get_tree_common(fc);
> +       mutex_lock(&cifs_mount_mutex);
> +       ret = smb3_get_tree_common(fc);
> +       mutex_unlock(&cifs_mount_mutex);
> +       return ret;
>  }
>
>  static void smb3_fs_context_free(struct fs_context *fc)
> --
> 2.30.2
>
Aurélien Aptel Jan. 20, 2022, 9:38 a.m. UTC | #2
On Wed, Jan 19, 2022 at 9:49 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ: 2008434
>
> If we try to perform multiple concurrent mounts ot the same server we might
> end up in a situation where:
> Thread #1                          Thread #2
>     creates TCP connection
>     Issues NegotiateProtocol
>     ...                            Pick the TCP connection for Thread #1
>                                    Issue a new NegotiateProtocol

checking server->tcpStatus state should prevent this situation no?

> which then leads to the the server kills off the session.
> There are also other a similar race where several threads ending up
> withe their own unique tcp connection that all go to the same server structure ....
>
> The most straightforward way to fix these races with concurrent mounts are to serialize
> them. I.e. only allow one mount to be in progress at a time.
diff mbox series

Patch

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index e3ed25dc6f3f..7ec35f3f0a5f 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -37,6 +37,8 @@ 
 #include "rfc1002pdu.h"
 #include "fs_context.h"
 
+static DEFINE_MUTEX(cifs_mount_mutex);
+
 static const match_table_t cifs_smb_version_tokens = {
 	{ Smb_1, SMB1_VERSION_STRING },
 	{ Smb_20, SMB20_VERSION_STRING},
@@ -707,10 +709,14 @@  static int smb3_get_tree_common(struct fs_context *fc)
 static int smb3_get_tree(struct fs_context *fc)
 {
 	int err = smb3_fs_context_validate(fc);
+	int ret;
 
 	if (err)
 		return err;
-	return smb3_get_tree_common(fc);
+	mutex_lock(&cifs_mount_mutex);
+	ret = smb3_get_tree_common(fc);
+	mutex_unlock(&cifs_mount_mutex);
+	return ret;
 }
 
 static void smb3_fs_context_free(struct fs_context *fc)