cifs: client: enforce consistent handling of multichannel and max_channels

Previously, the behavior of the multichannel and max_channels mount
options was inconsistent and order-dependent. For example, specifying
"multichannel,max_channels=1" would result in 2 channels, while
"max_channels=1,multichannel" would result in 1 channel. Additionally,
conflicting combinations such as "nomultichannel,max_channels=3" or
"multichannel,max_channels=1" did not produce errors and could lead to
unexpected channel counts.

This commit introduces two new fields in smb3_fs_context to explicitly
track whether multichannel and max_channels were specified during
mount. The option parsing and validation logic is updated to ensure:
- The outcome is no longer dependent on the order of options.
- Conflicting combinations (e.g., "nomultichannel,max_channels=3" or
  "multichannel,max_channels=1") are detected and result in an error.
- The number of channels created is consistent with the specified
  options.

This improves the reliability and predictability of mount option
handling for SMB3 multichannel support.

Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 6eccb9ed9daa38e19645aee29c5cd276fbc28872..71801d6bd4656ba9998160c7437ca37a0138c593 100644 (file)
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -1016,7 +1016,6 @@ cifs_smb3_do_mount(struct file_system_type *fs_type,
        } else {
                cifs_info("Attempting to mount %s\n", old_ctx->source);
        }
-
        cifs_sb = kzalloc(sizeof(*cifs_sb), GFP_KERNEL);
        if (!cifs_sb)
                return ERR_PTR(-ENOMEM);

diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 2a0d8b87bd8ea83653e126e578fb1c274e5a43ae..a966c6a8b1f5bf195c19ce359b7cde64bcb98b2b 100644 (file)
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -711,6 +711,47 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx)
        return 0;
 }
 
+static int smb3_handle_conflicting_options(struct fs_context *fc)
+{
+       struct smb3_fs_context *ctx = smb3_fc2context(fc);
+
+       if (ctx->multichannel_specified) {
+               if (ctx->multichannel) {
+                       if (!ctx->max_channels_specified) {
+                               ctx->max_channels = 2;
+                       } else if (ctx->max_channels == 1) {
+                               cifs_errorf(fc,
+                                           "max_channels must be greater than 1 when multichannel is enabled\n");
+                               return -EINVAL;
+                       }
+               } else {
+                       if (!ctx->max_channels_specified) {
+                               ctx->max_channels = 1;
+                       } else if (ctx->max_channels > 1) {
+                               cifs_errorf(fc,
+                                           "max_channels must be equal to 1 when multichannel is disabled\n");
+                               return -EINVAL;
+                       }
+               }
+       } else {
+               if (ctx->max_channels_specified) {
+                       if (ctx->max_channels > 1)
+                               ctx->multichannel = true;
+                       else
+                               ctx->multichannel = false;
+               } else {
+                       ctx->multichannel = false;
+                       ctx->max_channels = 1;
+               }
+       }
+
+       //resetting default values as remount doesn't initialize fs_context again
+       ctx->multichannel_specified = false;
+       ctx->max_channels_specified = false;
+
+       return 0;
+}
+
 static void smb3_fs_context_free(struct fs_context *fc);
 static int smb3_fs_context_parse_param(struct fs_context *fc,
                                       struct fs_parameter *param);
@@ -784,6 +825,7 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
                if (ret < 0)
                        break;
        }
+       ret = smb3_handle_conflicting_options(fc);
 
        return ret;
 }
@@ -1250,15 +1292,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
                ctx->nodelete = 1;
                break;
        case Opt_multichannel:
-               if (result.negated) {
+               ctx->multichannel_specified = true;
+               if (result.negated)
                        ctx->multichannel = false;
-                       ctx->max_channels = 1;
-               } else {
+               else
                        ctx->multichannel = true;
-                       /* if number of channels not specified, default to 2 */
-                       if (ctx->max_channels < 2)
-                               ctx->max_channels = 2;
-               }
                break;
        case Opt_uid:
                ctx->linux_uid = result.uid;
@@ -1394,15 +1432,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
                ctx->max_credits = result.uint_32;
                break;
        case Opt_max_channels:
+               ctx->max_channels_specified = true;
                if (result.uint_32 < 1 || result.uint_32 > CIFS_MAX_CHANNELS) {
                        cifs_errorf(fc, "%s: Invalid max_channels value, needs to be 1-%d\n",
                                 __func__, CIFS_MAX_CHANNELS);
                        goto cifs_parse_mount_err;
                }
                ctx->max_channels = result.uint_32;
-               /* If more than one channel requested ... they want multichan */
-               if (result.uint_32 > 1)
-                       ctx->multichannel = true;
                break;
        case Opt_max_cached_dirs:
                if (result.uint_32 < 1) {
@@ -1820,13 +1856,6 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
                goto cifs_parse_mount_err;
        }
 
-       /*
-        * Multichannel is not meaningful if max_channels is 1.
-        * Force multichannel to false to ensure consistent configuration.
-        */
-       if (ctx->multichannel && ctx->max_channels == 1)
-               ctx->multichannel = false;
-
        return 0;
 
  cifs_parse_mount_err:
@@ -1913,6 +1942,8 @@ int smb3_init_fs_context(struct fs_context *fc)
 
        /* default to no multichannel (single server connection) */
        ctx->multichannel = false;
+       ctx->multichannel_specified = false;
+       ctx->max_channels_specified = false;
        ctx->max_channels = 1;
 
        ctx->backupuid_specified = false; /* no backup intent for a user */

diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
index b0fec6b9a23b4fb8cc71061066493b0d6182a0de..7af7cbbe420884205f07418957ed836a1a095240 100644 (file)
--- a/fs/smb/client/fs_context.h
+++ b/fs/smb/client/fs_context.h
@@ -294,6 +294,8 @@ struct smb3_fs_context {
        bool domainauto:1;
        bool rdma:1;
        bool multichannel:1;
+       bool multichannel_specified:1; /* true if user specified multichannel or nomultichannel */
+       bool max_channels_specified:1; /* true if user specified max_channels */
        bool use_client_guid:1;
        /* reuse existing guid for multichannel */
        u8 client_guid[SMB2_CLIENT_GUID_SIZE];