[PATCH v2] smb/client: fix chan_max and UNC state corruption in smb3_reconfigure

[DaeMyung Kang]

smb3_reconfigure() has four related bugs when handling a
multichannel remount that leave ses->chan_max or cifs_sb->ctx
inconsistent with the actual state.

1) smb3_sync_ses_chan_max() is called before acquiring
   CIFS_SES_FLAG_SCALE_CHANNELS. If a concurrent operation (e.g.
   smb2_reconnect) holds the flag, the current thread takes the
   loser path and returns -EINVAL, but ses->chan_max has already
   been updated to the new value. chan_max is then inconsistent
   with the actual channel state.

2) When smb3_update_ses_channels() fails, ses->chan_max is not
   rolled back and the error is not propagated to the caller.
   mount returns success even though the channel configuration
   was not applied, and repeated failures cause chan_max to
   drift further from reality.

3) Earlier in smb3_reconfigure(), STEAL_STRING moves UNC, source
   and username from cifs_sb->ctx into ctx, setting
   cifs_sb->ctx->UNC to NULL until smb3_fs_context_dup() copies
   them back near the end of the function. Both the existing
   CIFS_SES_FLAG_SCALE_CHANNELS loser-path 'return -EINVAL' and a
   naive 'if (rc < 0) return rc;' to propagate the failure above
   exit in this window. A single failed multichannel remount
   therefore permanently nulls cifs_sb->ctx->UNC; /proc/mounts
   shows the device as "none" and every subsequent
   mount.cifs-based remount is rejected by
   smb3_verify_reconfigure_ctx() because mount.cifs passes that
   "none" back as the new UNC.

4) If the multichannel update fails but smb3_fs_context_dup() is
   still allowed to run with the new ctx values, the rejected
   new max_channels is written into cifs_sb->ctx, undoing the
   ses->chan_max rollback from (2) and creating a fresh
   cifs_sb->ctx vs ses->chan_max mismatch.

Fix all four by:
 - Moving smb3_sync_ses_chan_max() after the SCALE_CHANNELS
   acquire so the loser path cannot corrupt chan_max.
 - Capturing old_chan_max before the sync and restoring it on
   failure while still holding SCALE_CHANNELS so a concurrent
   reconfigure cannot race with the rollback.
 - Recording any multichannel-path failure in a local mchan_rc
   and routing it through a common 'out:' label so every exit
   reaches smb3_fs_context_dup() and cifs_sb->ctx is restored.
 - Before the dup, restoring ctx->multichannel and
   ctx->max_channels from cifs_sb->ctx on mchan_rc so the dup
   does not desync cifs_sb->ctx from the already-rolled-back
   ses->chan_max.
 - Returning mchan_rc to userspace so a failed scaling remount
   surfaces as an error.
 - Surfacing smb3_fs_context_dup() failure in preference to
   mchan_rc when both occur. The dup is the final step of the
   ctx restore and runs after smb3_cleanup_fs_context_contents()
   has already freed cifs_sb->ctx's strings, so a dup failure
   (e.g. -ENOMEM) leaves cifs_sb->ctx partially restored with
   some strings still NULL. Reporting the scaling error in that
   case would hide the real recovery status from userspace;
   reporting the dup error exposes it so the caller can react
   accordingly.

Note: smb3_reconfigure() is not fully transactional. Credentials
(ses->password and ses->password2) are committed before the
multichannel block, so a failed multichannel remount can still
leave ses->password updated while reporting an error to
userspace. Making the entire function atomic is a larger
refactor and is left for a separate patch.

Tested with a QEMU VM (ksmbd + cifs) using module-parameter-based
fault injection:
 - Forced smb3_update_ses_channels() failure via module param
   and verified ses->chan_max is preserved at the old value
   after remount failure.
 - Pre-set CIFS_SES_FLAG_SCALE_CHANNELS before entering the
   scaling path and verified the loser path returns -EINVAL
   without corrupting ses->chan_max.
 - Repeated 19 forced-failure remounts with varying max_channels
   (range 2-8) and confirmed no chan_max drift.
 - After each failure path, verified /proc/mounts continues to
   show the original UNC (//127.0.0.1/share) so subsequent
   remounts are accepted.
All scenarios pass with the patch and fail without it.

Reported-by: RAJASI MANDAL <rajasimandalos@xxxxxxxxx>
Closes: https://lore.kernel.org/lkml/CAEY6_V1+dzW3OD5zqXhsWyXwrDTrg5tAMGZ1AJ7_GAuRE+aevA@xxxxxxxxxxxxxx/
Fixes: ef529f655a2c ("cifs: client: allow changing multichannel mount options on remount")
Signed-off-by: DaeMyung Kang <charsyam@xxxxxxxxx>
---
 fs/smb/client/fs_context.c | 48 ++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index b9544eb0381b..9efd73ce6910 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -1085,10 +1085,11 @@ static int smb3_reconfigure(struct fs_context *fc)
 	struct dentry *root = fc->root;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb);
 	struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses;
+	unsigned int old_chan_max;
 	unsigned int rsize = ctx->rsize, wsize = ctx->wsize;
 	char *new_password = NULL, *new_password2 = NULL;
 	bool need_recon = false;
-	int rc;
+	int rc, mchan_rc = 0;
 
 	if (ses->expired_pwd)
 		need_recon = true;
@@ -1170,25 +1171,37 @@ static int smb3_reconfigure(struct fs_context *fc)
 	if ((ctx->multichannel != cifs_sb->ctx->multichannel) ||
 	    (ctx->max_channels != cifs_sb->ctx->max_channels)) {
 
-		/* Synchronize ses->chan_max with the new mount context */
-		smb3_sync_ses_chan_max(ses, ctx->max_channels);
-		/* Now update the session's channels to match the new configuration */
 		/* Prevent concurrent scaling operations */
 		spin_lock(&ses->ses_lock);
 		if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) {
 			spin_unlock(&ses->ses_lock);
 			mutex_unlock(&ses->session_mutex);
-			return -EINVAL;
+			mchan_rc = -EINVAL;
+			goto out;
 		}
 		ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS;
 		spin_unlock(&ses->ses_lock);
 
+		old_chan_max = ses->chan_max;
+		/* Synchronize ses->chan_max with the new mount context */
+		smb3_sync_ses_chan_max(ses, ctx->max_channels);
+
 		mutex_unlock(&ses->session_mutex);
 
 		rc = smb3_update_ses_channels(ses, ses->server,
 					       false /* from_reconnect */,
 					       false /* disable_mchan */);
 
+		/*
+		 * On failure, restore chan_max while still holding
+		 * CIFS_SES_FLAG_SCALE_CHANNELS so a concurrent reconfigure
+		 * cannot observe or race with the rollback.
+		 */
+		if (rc < 0) {
+			smb3_sync_ses_chan_max(ses, old_chan_max);
+			mchan_rc = rc;
+		}
+
 		/* Clear scaling flag after operation */
 		spin_lock(&ses->ses_lock);
 		ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS;
@@ -1197,6 +1210,7 @@ static int smb3_reconfigure(struct fs_context *fc)
 		mutex_unlock(&ses->session_mutex);
 	}
 
+out:
 	STEAL_STRING(cifs_sb, ctx, domainname);
 	STEAL_STRING(cifs_sb, ctx, nodename);
 	STEAL_STRING(cifs_sb, ctx, iocharset);
@@ -1205,12 +1219,32 @@ static int smb3_reconfigure(struct fs_context *fc)
 	ctx->rsize = rsize ? CIFS_ALIGN_RSIZE(fc, rsize) : cifs_sb->ctx->rsize;
 	ctx->wsize = wsize ? CIFS_ALIGN_WSIZE(fc, wsize) : cifs_sb->ctx->wsize;
 
+	/*
+	 * If the multichannel update failed, restore the old multichannel
+	 * settings in ctx so smb3_fs_context_dup() does not desync
+	 * cifs_sb->ctx from ses->chan_max (which was already rolled back).
+	 */
+	if (mchan_rc) {
+		ctx->multichannel = cifs_sb->ctx->multichannel;
+		ctx->max_channels = cifs_sb->ctx->max_channels;
+	}
+
 	smb3_cleanup_fs_context_contents(cifs_sb->ctx);
 	rc = smb3_fs_context_dup(cifs_sb->ctx, ctx);
 	smb3_update_mnt_flags(cifs_sb);
+
+	/*
+	 * If smb3_fs_context_dup() failed, cifs_sb->ctx is left in a
+	 * partially-restored state after the cleanup above. Surface that
+	 * failure to the caller before any mchan_rc, since it reflects the
+	 * real recovery status of cifs_sb->ctx rather than the scaling error.
+	 */
+	if (rc)
+		return rc;
+	if (mchan_rc)
+		return mchan_rc;
 #ifdef CONFIG_CIFS_DFS_UPCALL
-	if (!rc)
-		rc = dfs_cache_remount_fs(cifs_sb);
+	rc = dfs_cache_remount_fs(cifs_sb);
 #endif
 
 	return rc;
-- 
2.43.0