From: rajasimandalos@gmail.com
To: linux-cifs@vger.kernel.org
Cc: smfrench@gmail.com, pc@manguebit.org, sprasad@microsoft.com,
bharathsm@microsoft.com, enzo@kernel.org
Subject: [PATCH v2 1/8] smb: client: block non-reconfigurable option changes on remount
Date: Thu, 7 May 2026 13:44:41 +0000 [thread overview]
Message-ID: <20260507134448.168602-2-rajasimandalos@gmail.com> (raw)
In-Reply-To: <20260507134448.168602-1-rajasimandalos@gmail.com>
From: Rajasi Mandal <rajasimandal@microsoft.com>
Several mount options (seal, sign, vers, ip, rdma, nosharesock,
persistent/resilient handles, etc.) require tearing down the
connection to take effect, but smb3_verify_reconfigure_ctx()
does not reject them. A remount that changes any of these silently
ignores the new value, confusing users.
Adding simple != checks to smb3_verify_reconfigure_ctx() does not
work with the current code because smb3_init_fs_context() always
creates a fresh context with init defaults on remount. Since
cifs_show_options() reads many fields from tcon/server/ses rather
than from ctx, the init defaults differ from the runtime state
and every != check would spuriously fire.
Fix this by detecting FS_CONTEXT_FOR_RECONFIGURE in
smb3_init_fs_context() and duplicating the existing cifs_sb->ctx
instead of building one from scratch. Before duplicating, sync
ctx with runtime state via new helper smb3_sync_ctx_from_tcon()
so the baseline matches what cifs_show_options() displays. With
this in place, add comprehensive checks in
smb3_verify_reconfigure_ctx() to reject non-reconfigurable option
changes with clear error messages.
Also preserve inherited multichannel/max_channels values in
smb3_handle_conflicting_options() during reconfigure when neither
option is explicitly specified.
Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
---
fs/smb/client/fs_context.c | 254 ++++++++++++++++++++++++++++++++++++-
1 file changed, 252 insertions(+), 2 deletions(-)
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index b63ec7ab6e51..d90430e7a648 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -717,13 +717,13 @@ static int smb3_handle_conflicting_options(struct fs_context *fc)
ctx->multichannel = true;
else
ctx->multichannel = false;
- } else {
+ } else if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE) {
ctx->multichannel = false;
ctx->max_channels = 1;
}
+ /* For reconfigure with neither specified, keep inherited values */
}
- //resetting default values as remount doesn't initialize fs_context again
ctx->multichannel_specified = false;
ctx->max_channels_specified = false;
@@ -921,6 +921,85 @@ static void smb3_fs_context_free(struct fs_context *fc)
smb3_cleanup_fs_context(ctx);
}
+/*
+ * Sync cifs_sb->ctx with the runtime state visible through /proc/mounts.
+ * cifs_show_options() reads many fields from tcon/server/ses rather than
+ * from ctx. On remount, libmount reads /proc/mounts and feeds those
+ * values back as mount options. To avoid false mismatches between the
+ * old ctx and the newly parsed options, we must ensure ctx reflects
+ * the current runtime state before it is duplicated into the new
+ * remount context.
+ *
+ * Note: sb->s_umount is not yet held when VFS calls init_fs_context()
+ * for reconfigure, so in theory concurrent I/O paths could read
+ * cifs_sb->ctx fields (e.g. cifs_symlink_type()) while we write them.
+ * This is safe because all fields written here are word-sized
+ * (bool/int/pointer), so stores are architecturally atomic. The same
+ * unsynchronized-read pattern already exists in cifs_show_options().
+ */
+static void smb3_sync_ctx_from_tcon(struct cifs_sb_info *cifs_sb)
+{
+ struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
+ struct TCP_Server_Info *server = tcon->ses->server;
+ struct cifs_ses *ses = tcon->ses;
+ struct smb3_fs_context *ctx = cifs_sb->ctx;
+ const char *domain;
+ int unicode;
+
+ /*
+ * Server fields: ops/vals can change during reconnect
+ * (renegotiation may upgrade the dialect). nosharesock can
+ * transition false->true if the server sets
+ * SMB2_SHAREFLAG_ISOLATED_TRANSPORT during tree connect.
+ * Read all under srv_lock to get a consistent snapshot.
+ */
+ spin_lock(&server->srv_lock);
+ ctx->ops = server->ops;
+ ctx->vals = server->vals;
+ ctx->nosharesock = server->nosharesock;
+ spin_unlock(&server->srv_lock);
+
+ /*
+ * Tcon fields: posix_extensions, unix_ext, use_persistent and
+ * use_resilient are set during tree connect and do not change
+ * after that, but read under tc_lock for consistency with the
+ * convention that tc_lock protects tcon state.
+ */
+ spin_lock(&tcon->tc_lock);
+ if (tcon->posix_extensions) {
+ ctx->linux_ext = 1;
+ ctx->no_linux_ext = 0;
+ } else if (tcon->unix_ext) {
+ ctx->linux_ext = 1;
+ ctx->no_linux_ext = 0;
+ } else {
+ ctx->linux_ext = 0;
+ ctx->no_linux_ext = 1;
+ }
+ if (tcon->use_persistent) {
+ ctx->persistent = true;
+ ctx->nopersistent = false;
+ }
+ ctx->resilient = tcon->use_resilient;
+ spin_unlock(&tcon->tc_lock);
+
+ /*
+ * Session fields: domainName and unicode are effectively
+ * write-once (set during session setup, never freed/replaced
+ * while the session exists). Snapshot them under ses_lock
+ * and kstrdup the domain outside the lock.
+ */
+ spin_lock(&ses->ses_lock);
+ domain = ses->domainName;
+ unicode = ses->unicode;
+ spin_unlock(&ses->ses_lock);
+
+ if (domain && !ctx->domainname)
+ ctx->domainname = kstrdup(domain, GFP_KERNEL);
+ if (unicode >= 0)
+ ctx->unicode = unicode;
+}
+
/*
* Compare the old and new proposed context during reconfigure
* and check if the changes are compatible.
@@ -990,6 +1069,143 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
cifs_errorf(fc, "can not change nbsessinit during remount\n");
return -EINVAL;
}
+ if (new_ctx->compress != old_ctx->compress) {
+ cifs_errorf(fc, "can not change compress during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->noblocksnd != old_ctx->noblocksnd) {
+ cifs_errorf(fc, "can not change noblocksend during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->noautotune != old_ctx->noautotune) {
+ cifs_errorf(fc, "can not change noautotune during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->no_sparse != old_ctx->no_sparse) {
+ cifs_errorf(fc, "can not change nosparse during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->nodelete != old_ctx->nodelete) {
+ cifs_errorf(fc, "can not change nodelete during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->cruid_specified &&
+ !uid_eq(new_ctx->cred_uid, old_ctx->cred_uid)) {
+ cifs_errorf(fc, "can not change cruid during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->port != old_ctx->port) {
+ cifs_errorf(fc, "can not change port during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->min_offload != old_ctx->min_offload) {
+ cifs_errorf(fc, "can not change min_enc_offload during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->snapshot_time != old_ctx->snapshot_time) {
+ cifs_errorf(fc, "can not change snapshot during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->max_credits != old_ctx->max_credits) {
+ cifs_errorf(fc, "can not change max_credits during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->handle_timeout != old_ctx->handle_timeout) {
+ cifs_errorf(fc, "can not change handletimeout during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->got_ip &&
+ !cifs_match_ipaddr((struct sockaddr *)&new_ctx->dstaddr,
+ (struct sockaddr *)&old_ctx->dstaddr)) {
+ cifs_errorf(fc, "can not change ip during remount\n");
+ return -EINVAL;
+ }
+ if (((struct sockaddr *)&new_ctx->srcaddr)->sa_family != AF_UNSPEC &&
+ memcmp(&new_ctx->srcaddr, &old_ctx->srcaddr, sizeof(new_ctx->srcaddr))) {
+ cifs_errorf(fc, "can not change srcaddr during remount\n");
+ return -EINVAL;
+ }
+ if (memcmp(new_ctx->source_rfc1001_name, old_ctx->source_rfc1001_name,
+ RFC1001_NAME_LEN)) {
+ cifs_errorf(fc, "can not change netbiosname during remount\n");
+ return -EINVAL;
+ }
+ if (memcmp(new_ctx->target_rfc1001_name, old_ctx->target_rfc1001_name,
+ RFC1001_NAME_LEN)) {
+ cifs_errorf(fc, "can not change servern during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->got_version &&
+ (new_ctx->ops != old_ctx->ops || new_ctx->vals != old_ctx->vals)) {
+ cifs_errorf(fc, "can not change vers during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->witness != old_ctx->witness) {
+ cifs_errorf(fc, "can not change witness during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->rootfs != old_ctx->rootfs) {
+ cifs_errorf(fc, "can not change rootfs during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->linux_ext != old_ctx->linux_ext ||
+ new_ctx->no_linux_ext != old_ctx->no_linux_ext) {
+ cifs_errorf(fc, "can not change unix during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->nocase != old_ctx->nocase) {
+ cifs_errorf(fc, "can not change nocase during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->intr != old_ctx->intr) {
+ cifs_errorf(fc, "can not change intr during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->no_psx_acl != old_ctx->no_psx_acl) {
+ cifs_errorf(fc, "can not change acl during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->local_lease != old_ctx->local_lease) {
+ cifs_errorf(fc, "can not change locallease during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->sign != old_ctx->sign) {
+ cifs_errorf(fc, "can not change sign during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->ignore_signature != old_ctx->ignore_signature) {
+ cifs_errorf(fc, "can not change ignore_signature during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->seal != old_ctx->seal) {
+ cifs_errorf(fc, "can not change seal during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->nosharesock != old_ctx->nosharesock) {
+ cifs_errorf(fc, "can not change nosharesock during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->persistent != old_ctx->persistent ||
+ new_ctx->nopersistent != old_ctx->nopersistent) {
+ cifs_errorf(fc, "can not change persistenthandles during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->resilient != old_ctx->resilient) {
+ cifs_errorf(fc, "can not change resilienthandles during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->sockopt_tcp_nodelay != old_ctx->sockopt_tcp_nodelay) {
+ cifs_errorf(fc, "can not change tcpnodelay during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->domainauto != old_ctx->domainauto) {
+ cifs_errorf(fc, "can not change domainauto during remount\n");
+ return -EINVAL;
+ }
+ if (new_ctx->rdma != old_ctx->rdma) {
+ cifs_errorf(fc, "can not change rdma during remount\n");
+ return -EINVAL;
+ }
return 0;
}
@@ -1903,6 +2119,40 @@ int smb3_init_fs_context(struct fs_context *fc)
char *nodename = utsname()->nodename;
int i;
+ /*
+ * For reconfigure (remount), duplicate the existing mount context
+ * instead of building one from scratch with init defaults.
+ *
+ * VFS sets fc->root before calling init_fs_context for reconfigure,
+ * so we can access the existing superblock's context. We first sync
+ * cifs_sb->ctx with runtime state (tcon/server/ses) so that ctx
+ * matches what cifs_show_options() displays. Then we dup old_ctx
+ * into new_ctx. The parser will overwrite only the options
+ * explicitly passed on remount, so any difference between new_ctx
+ * and old_ctx in smb3_verify_reconfigure_ctx() represents a real,
+ * intentional change by the user.
+ */
+ if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+ struct cifs_sb_info *cifs_sb = CIFS_SB(fc->root->d_sb);
+ int rc;
+
+ smb3_sync_ctx_from_tcon(cifs_sb);
+
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ rc = smb3_fs_context_dup(ctx, cifs_sb->ctx);
+ if (rc) {
+ kfree(ctx);
+ return rc;
+ }
+
+ fc->fs_private = ctx;
+ fc->ops = &smb3_fs_context_ops;
+ return 0;
+ }
+
ctx = kzalloc_obj(struct smb3_fs_context);
if (unlikely(!ctx))
return -ENOMEM;
--
2.43.0
next prev parent reply other threads:[~2026-05-07 13:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-07 13:44 [PATCH v2 0/8] Remount patches v2 rajasimandalos
2026-05-07 13:44 ` rajasimandalos [this message]
2026-05-07 13:44 ` [PATCH v2 2/8] smb: client: sync tcon-level options on remount rajasimandalos
2026-05-07 13:44 ` [PATCH v2 3/8] smb: client: sync retrans " rajasimandalos
2026-05-07 13:44 ` [PATCH v2 4/8] smb: client: sync echo_interval " rajasimandalos
2026-05-07 13:44 ` [PATCH v2 5/8] smb: client: move struct tcon_list to cifsglob.h rajasimandalos
2026-05-07 13:44 ` [PATCH v2 6/8] smb: client: allow nolease option to be reconfigured on remount rajasimandalos
2026-05-07 13:44 ` [PATCH v2 7/8] smb: client: block cache=ro and cache=singleclient " rajasimandalos
2026-05-07 13:44 ` [PATCH v2 8/8] smb: client: apply rasize " rajasimandalos
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260507134448.168602-2-rajasimandalos@gmail.com \
--to=rajasimandalos@gmail.com \
--cc=bharathsm@microsoft.com \
--cc=enzo@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=pc@manguebit.org \
--cc=smfrench@gmail.com \
--cc=sprasad@microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox