[PATCH v3] smb/client: fix state corruption in smb3_reconfigure multichannel path
From: DaeMyung Kang
Date: Thu Apr 16 2026 - 16:45:51 EST
smb3_reconfigure() has several state-consistency bugs when handling
a multichannel remount that leave ses->chan_max or cifs_sb->ctx
inconsistent with the actual state. This patch repairs internal
state only; the userspace-visible return value of
smb3_reconfigure() is preserved to match the pre-patch behaviour:
the concurrent-scale loser path still returns -EINVAL, and a
failed smb3_update_ses_channels() is not newly propagated to
userspace by this patch.
Bugs addressed:
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 out of sync
with the actual channel state.
2) When smb3_update_ses_channels() fails, ses->chan_max is not
rolled back. Repeated failures cause chan_max to drift further
from reality, and subsequent reconnect/reconfigure paths use
the drifted target.
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. The pre-existing
CIFS_SES_FLAG_SCALE_CHANNELS loser-path 'return -EINVAL' exits
inside this window. A loser-path failure 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) Once (2) rolls ses->chan_max back to the old value on update
failure, smb3_fs_context_dup() would still copy the rejected
new ctx->max_channels into cifs_sb->ctx, creating a fresh
cifs_sb->ctx vs ses->chan_max mismatch. This must be handled
together with (2) to keep the rollback complete.
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 the loser path through a common 'out:' label so
every exit reaches smb3_fs_context_dup() and cifs_sb->ctx is
restored. mchan_rc is used only as internal control flow.
- 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.
- Tracking the concurrent-scale loser path with a separate
scale_busy flag. After cifs_sb->ctx is fully restored, the
return value is forced to -EINVAL for that path so userspace
continues to see the pre-patch rejection. dup /
dfs_cache_remount_fs() failures still take precedence because
they reflect real state recovery errors.
Deliberately not changed by this patch: the return value of
smb3_reconfigure() on smb3_update_ses_channels() failure. That
path is not newly propagated to userspace here, matching the
asynchronous model used by the mount path (mchan_mount_work_fn).
Those return-semantics and deferred-handling questions are left
to follow-up discussion/patches.
smb3_reconfigure() is not fully transactional: ses->password and
ses->password2 are committed before the multichannel block, so
unrelated earlier state changes may still be visible after a
failed multichannel remount. That is a structural property of
the function and out of scope here.
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 the remount path runs.
- Pre-set CIFS_SES_FLAG_SCALE_CHANNELS before entering the
scaling path and verified the loser path still returns
-EINVAL, no longer corrupts ses->chan_max, and no longer nulls
cifs_sb->ctx->UNC.
- 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.
Reported-by: RAJASI MANDAL <rajasimandalos@xxxxxxxxx>
Closes: https://lore.kernel.org/lkml/CAEY6_V1+dzW3OD5zqXhsWyXwrDTrg5tAMGZ1AJ7_GAuRE+aevA@xxxxxxxxxxxxxx/
Link: https://lore.kernel.org/lkml/xkr2dlvgibq5j6gkcxd3yhhnj4atgxw2uy4eug2pxm7wy7nbms@iq6cf5taa65v/
Fixes: ef529f655a2c ("cifs: client: allow changing multichannel mount options on remount")
Signed-off-by: DaeMyung Kang <charsyam@xxxxxxxxx>
---
v3: (feedback from Henrique Carvalho)
- Drop propagation of smb3_update_ses_channels() failure to
userspace; preserve the pre-patch best-effort semantics on that
path.
- Keep the pre-existing CIFS_SES_FLAG_SCALE_CHANNELS loser-path
-EINVAL via a separate scale_busy flag so that return is not
silently converted into success.
- Reword commit message to describe accurately what userspace-
visible semantics are preserved and what changed internally.
v2: (feedback from Rajasi Mandal)
- Route loser-path and update-failure exits through a common
'out:' label so smb3_fs_context_dup() always runs and
cifs_sb->ctx->UNC is restored after STEAL_STRING.
- Restore ctx->multichannel/ctx->max_channels from cifs_sb->ctx
before the dup so dup does not re-desync cifs_sb->ctx from the
rolled-back ses->chan_max.
fs/smb/client/fs_context.c | 46 +++++++++++++++++++++++++++++++++-----
1 file changed, 41 insertions(+), 5 deletions(-)
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index b9544eb0381b..aaa364a3f60d 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -1085,10 +1085,12 @@ 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;
+ bool scale_busy = false;
+ int rc, mchan_rc = 0;
if (ses->expired_pwd)
need_recon = true;
@@ -1170,25 +1172,38 @@ 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;
+ scale_busy = true;
+ 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 +1212,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,6 +1221,16 @@ 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);
@@ -1213,6 +1239,16 @@ static int smb3_reconfigure(struct fs_context *fc)
rc = dfs_cache_remount_fs(cifs_sb);
#endif
+ /*
+ * Preserve the pre-existing loser-path semantics: a concurrent
+ * scaling operation causes the remount to be rejected with
+ * -EINVAL. smb3_fs_context_dup() / dfs_cache_remount_fs()
+ * failures take precedence because they reflect real state
+ * recovery errors. Other multichannel failures remain best-effort.
+ */
+ if (!rc && scale_busy)
+ rc = -EINVAL;
+
return rc;
}
--
2.43.0