From: Rajasi Mandal <[email protected]>
smb3_init_fs_context() builds a fresh context from init defaults on
every call, including remount. Many options shown by
cifs_show_options() come from tcon/server/ses rather than ctx, so those
defaults don't reflect the live mount and make a poor baseline for
deciding what changed on remount.
On reconfigure, duplicate the live cifs_sb->ctx into a private context
and sync it with runtime state via a new smb3_sync_ctx_from_runtime()
so the baseline matches what /proc/mounts displays. This mirrors
libmount (util-linux), which on remount replays the current
/proc/mounts options back to the kernel: the baseline must equal the
displayed state, or a bare 'mount -o remount' looks like a change and
is rejected. libmount isn't present everywhere, but the kernel must
accept such a replay regardless. Syncing into the private copy (and
comparing against a freshly synced base_ctx in smb3_reconfigure())
avoids racing /proc/mounts readers; wide fields (dstaddr, ops/vals) are
read under srv_lock and tcon->unix_ext under tc_lock.
Only positive runtime state is reflected: no_linux_ext / nopersistent
are not fabricated from inactive runtime state, as a fabricated value
committed to cifs_sb->ctx would suppress POSIX / persistent-handle
auto-enable on a later reconnect or failover.
This is preparatory plumbing for the next patch, which compares new_ctx
against old_ctx to reject non-reconfigurable option changes.
Not addressed (pre-existing): this race is not introduced here.
smb3_reconfigure() already replaces cifs_sb->ctx in place with
smb3_cleanup_fs_context_contents(cifs_sb->ctx);
memcpy(cifs_sb->ctx, new_ctx, sizeof(*new_ctx));
while cifs_show_options() has always read the same cifs_sb->ctx
locklessly for /proc/mounts, so a reader racing a remount of that mount
can already observe a torn field or a briefly stale string pointer.
The risk is low: remount is an infrequent, admin-triggered operation
serialized by s_umount, the only concurrent reader is the diagnostic
/proc/mounts path, and the worst outcome is a transiently inconsistent
options line for that one mount rather than memory corruption or a
crash. This patch neither widens that window nor adds a new lockless
reader -- init dup()s cifs_sb->ctx the same way the mount path already
does -- so a proper fix (RCU-style ctx replacement) is left as a
separate change.
Signed-off-by: Rajasi Mandal <[email protected]>
---
fs/smb/client/cifs_swn.h | 14 ++-
fs/smb/client/fs_context.c | 197 ++++++++++++++++++++++++++++++++++++-
fs/smb/client/smb1ops.c | 7 +-
fs/smb/client/smb2pdu.c | 11 ++-
4 files changed, 223 insertions(+), 6 deletions(-)
diff --git a/fs/smb/client/cifs_swn.h b/fs/smb/client/cifs_swn.h
index 955d07b69450..caf0779d4d07 100644
--- a/fs/smb/client/cifs_swn.h
+++ b/fs/smb/client/cifs_swn.h
@@ -26,11 +26,21 @@ void cifs_swn_check(void);
static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server)
{
+ bool ret = false;
+
+ /*
+ * srv_lock serializes the 128-byte sockaddr_storage write against
+ * concurrent readers (e.g. cifs_show_address(), reconn_set_ipaddr_
+ * from_hostname() snapshot, smb3_sync_ctx_from_runtime()) and other
+ * writers like cifs_chan_update_iface() which already use srv_lock.
+ */
+ spin_lock(&server->srv_lock);
if (server->use_swn_dstaddr) {
server->dstaddr = server->swn_dstaddr;
- return true;
+ ret = true;
}
- return false;
+ spin_unlock(&server->srv_lock);
+ return ret;
}
static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server)
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index abc29500a664..1a68ae058166 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -926,6 +926,121 @@ static void smb3_fs_context_free(struct fs_context *fc)
smb3_cleanup_fs_context(ctx);
}
+/*
+ * Sync a private reconfigure context with runtime state from
+ * tcon/server/ses so the baseline matches what cifs_show_options()
+ * displays. @ctx must be the caller's freshly-duplicated context, not
+ * the live cifs_sb->ctx: this function writes into @ctx, and mutating
+ * the shared cifs_sb->ctx here would race with concurrent fspick/remount
+ * and /proc/mounts readers. Wide fields (dstaddr, ops/vals) are read
+ * under the matching server/tcon lock; the remaining word-sized scalars
+ * rely on the same unsynchronized-read pattern already used by
+ * cifs_show_options().
+ */
+static int smb3_sync_ctx_from_runtime(struct cifs_sb_info *cifs_sb,
+ struct smb3_fs_context *ctx)
+{
+ struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
+ struct TCP_Server_Info *server = tcon->ses->server;
+ struct cifs_ses *ses = tcon->ses;
+ const char *domain;
+ int unicode;
+
+ /*
+ * Server fields that can drift from ctx after mount:
+ * - ops/vals: dialect renegotiation during reconnect (paired,
+ * so read under srv_lock to match the writer in SMB2_negotiate)
+ * - dstaddr: SWN witness failover updates server->dstaddr; the
+ * 128-byte sockaddr_storage is not atomic, so srv_lock is
+ * required against torn reads
+ * - nosharesock: can be flipped to true post-mount by SMB2_tcon
+ * on STATUS_BAD_NETWORK_NAME with ISOLATED_TRANSPORT, so read
+ * under srv_lock to pair with that writer
+ */
+ spin_lock(&server->srv_lock);
+ ctx->ops = server->ops;
+ ctx->vals = server->vals;
+ ctx->dstaddr = server->dstaddr;
+ ctx->nosharesock = server->nosharesock;
+ spin_unlock(&server->srv_lock);
+
+ /*
+ * These connection-tied options live on the shared TCP_Server_Info and
+ * are reported by cifs_show_options() from server->*, not from the
+ * per-mount ctx. On a connection shared by several mounts, a mount that
+ * never set them still displays them in /proc/mounts, so a
+ * `mount -a -o remount` replays them into the new ctx. Sync them from
+ * the live server into the baseline so such an otherwise bare remount is
+ * not rejected by smb3_verify_reconfigure_ctx(). They are established at
+ * mount time and not mutated at runtime, so plain reads match how both
+ * cifs_show_options() and cifs_construct_tcon() read them.
+ */
+ ctx->noblocksnd = server->noblocksnd;
+ ctx->noautotune = server->noautotune;
+ ctx->sockopt_tcp_nodelay = server->tcp_nodelay;
+ ctx->echo_interval = server->echo_interval / HZ;
+ ctx->max_credits = server->max_credits;
+ ctx->min_offload = server->min_offload;
+
+ /*
+ * tcon->unix_ext can be flipped post-mount by reset_cifs_unix_caps()
+ * on SMB1 reconnect (smb1_reconnect path). Read under tc_lock to pair
+ * with that writer. tcon->posix_extensions is only ever set at
+ * mount-time pre-publish, but read it under the same lock so the
+ * derived linux_ext value is consistent.
+ *
+ * Only reflect the positive runtime state (extensions active) here. Do
+ * NOT fabricate no_linux_ext when they are inactive: no_linux_ext is the
+ * user's explicit "nounix" request, whereas an inactive runtime state may
+ * merely mean the current server does not offer them. This ctx is copied
+ * into cifs_sb->ctx by smb3_reconfigure(), so a fabricated no_linux_ext
+ * would wrongly suppress POSIX auto-enable on a later reconnect/failover
+ * to a capable server -- the same reason nopersistent is not derived from
+ * use_persistent below.
+ */
+ spin_lock(&tcon->tc_lock);
+ if (tcon->posix_extensions || tcon->unix_ext) {
+ ctx->linux_ext = 1;
+ ctx->no_linux_ext = 0;
+ }
+ spin_unlock(&tcon->tc_lock);
+ ctx->seal = tcon->seal;
+ /*
+ * persistent reflects the live tcon->use_persistent so the baseline
+ * matches what cifs_show_options() reports (persistenthandles). Do
+ * NOT derive nopersistent from use_persistent: persistent and
+ * nopersistent are independent user options and use_persistent is a
+ * derived runtime result. nopersistent is not reported by
+ * cifs_show_options(), so it must keep the user's original value
+ * (carried by the dup); fabricating it here would, after the
+ * reconfigure commits the ctx, wrongly suppress persistent-handle
+ * auto-enable on continuous-availability shares at reconnect.
+ */
+ ctx->persistent = tcon->use_persistent;
+ ctx->resilient = tcon->use_resilient;
+ ctx->witness = tcon->use_witness;
+
+ /*
+ * Session fields: domainName and unicode are effectively
+ * write-once (set during session setup, never freed/replaced
+ * while the session exists), so plain reads are safe. @ctx is
+ * the caller's private copy, so filling in a missing domainname
+ * here cannot leak or race.
+ */
+ domain = ses->domainName;
+ unicode = ses->unicode;
+
+ if (domain && !ctx->domainname) {
+ ctx->domainname = kstrdup(domain, GFP_KERNEL);
+ if (!ctx->domainname)
+ return -ENOMEM;
+ }
+ if (unicode >= 0)
+ ctx->unicode = unicode;
+
+ return 0;
+}
+
/*
* Compare the old and new proposed context during reconfigure
* and check if the changes are compatible.
@@ -1067,6 +1182,7 @@ static int smb3_reconfigure(struct fs_context *fc)
struct smb3_fs_context *ctx = smb3_fc2context(fc);
struct smb3_fs_context *new_ctx = NULL;
struct smb3_fs_context *old_ctx = NULL;
+ struct smb3_fs_context *base_ctx = NULL;
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;
@@ -1079,9 +1195,37 @@ static int smb3_reconfigure(struct fs_context *fc)
if (ses->expired_pwd)
need_recon = true;
- rc = smb3_verify_reconfigure_ctx(fc, ctx, cifs_sb->ctx, need_recon);
+ /*
+ * Compare the new context against a runtime-synced baseline, not the
+ * raw cifs_sb->ctx. smb3_init_fs_context() syncs the new context
+ * (fc->fs_private) from runtime state, so the comparison baseline must
+ * be synced the same way; otherwise a field that drifted at runtime
+ * (e.g. dstaddr after witness failover, or use_persistent auto-enabled
+ * on a continuous-availability share) would look like a user-requested
+ * change and be rejected on an otherwise bare remount. Sync a private
+ * copy rather than cifs_sb->ctx: s_umount serializes reconfigures, but
+ * not the fspick/fsconfig init path, which dup()s cifs_sb->ctx in
+ * smb3_init_fs_context() before reconfigure_super() takes s_umount. An
+ * in-place sync would therefore race that lockless dup (e.g. a torn
+ * read of the 128-byte dstaddr).
+ */
+ base_ctx = kzalloc_obj(*base_ctx);
+ if (!base_ctx)
+ return -ENOMEM;
+ rc = smb3_fs_context_dup(base_ctx, cifs_sb->ctx);
if (rc)
- return rc;
+ goto free_base_ctx;
+ rc = smb3_sync_ctx_from_runtime(cifs_sb, base_ctx);
+ if (rc)
+ goto cleanup_base_ctx;
+
+ rc = smb3_verify_reconfigure_ctx(fc, ctx, base_ctx, need_recon);
+ if (rc)
+ goto cleanup_base_ctx;
+
+ smb3_cleanup_fs_context_contents(base_ctx);
+ kfree(base_ctx);
+ base_ctx = NULL;
rc = smb3_handle_conflicting_options(fc);
if (rc)
@@ -1245,6 +1389,13 @@ static int smb3_reconfigure(struct fs_context *fc)
free_old_ctx:
kfree(old_ctx);
+ return rc;
+
+cleanup_base_ctx:
+ smb3_cleanup_fs_context_contents(base_ctx);
+free_base_ctx:
+ kfree(base_ctx);
+
return rc;
}
@@ -1961,6 +2112,48 @@ 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 dup the
+ * live cifs_sb->ctx into a private new_ctx, then sync new_ctx with
+ * runtime state (tcon/server/ses) so it matches what
+ * cifs_show_options() displays. Syncing into the private copy (not
+ * the shared cifs_sb->ctx) avoids racing with concurrent
+ * fspick/remount and /proc/mounts readers. 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;
+
+ ctx = kzalloc_obj(struct smb3_fs_context);
+ if (!ctx)
+ return -ENOMEM;
+
+ rc = smb3_fs_context_dup(ctx, cifs_sb->ctx);
+ if (rc) {
+ smb3_cleanup_fs_context_contents(ctx);
+ kfree(ctx);
+ return rc;
+ }
+
+ rc = smb3_sync_ctx_from_runtime(cifs_sb, ctx);
+ if (rc) {
+ smb3_cleanup_fs_context_contents(ctx);
+ 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;
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index dc5a8c1da623..16df7c423a9b 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -36,11 +36,16 @@ void reset_cifs_unix_caps(unsigned int xid, struct cifs_tcon *tcon,
if (ctx && ctx->no_linux_ext) {
tcon->fsUnixInfo.Capability = 0;
+ spin_lock(&tcon->tc_lock);
tcon->unix_ext = 0; /* Unix Extensions disabled */
+ spin_unlock(&tcon->tc_lock);
cifs_dbg(FYI, "Linux protocol extensions disabled\n");
return;
- } else if (ctx)
+ } else if (ctx) {
+ spin_lock(&tcon->tc_lock);
tcon->unix_ext = 1; /* Unix Extensions supported */
+ spin_unlock(&tcon->tc_lock);
+ }
if (!tcon->unix_ext) {
cifs_dbg(FYI, "Unix extensions disabled so not set on reconnect\n");
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 4ce165e40657..ee63b8547728 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -1215,8 +1215,10 @@ SMB2_negotiate(const unsigned int xid,
goto neg_exit;
case SMB311_PROT_ID:
/* ops set to 3.0 by default for default so update */
+ spin_lock(&server->srv_lock);
server->ops = &smb311_operations;
server->vals = &smb311_values;
+ spin_unlock(&server->srv_lock);
break;
default:
break;
@@ -1231,12 +1233,16 @@ SMB2_negotiate(const unsigned int xid,
goto neg_exit;
case SMB21_PROT_ID:
/* ops set to 3.0 by default for default so update */
+ spin_lock(&server->srv_lock);
server->ops = &smb21_operations;
server->vals = &smb21_values;
+ spin_unlock(&server->srv_lock);
break;
case SMB311_PROT_ID:
+ spin_lock(&server->srv_lock);
server->ops = &smb311_operations;
server->vals = &smb311_values;
+ spin_unlock(&server->srv_lock);
break;
default:
break;
@@ -2252,8 +2258,11 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
if (server->ops->validate_negotiate)
rc = server->ops->validate_negotiate(xid, tcon);
if (rc == 0) /* See MS-SMB2 2.2.10 and 3.2.5.5 */
- if (tcon->share_flags & SMB2_SHAREFLAG_ISOLATED_TRANSPORT)
+ if (tcon->share_flags & SMB2_SHAREFLAG_ISOLATED_TRANSPORT) {
+ spin_lock(&server->srv_lock);
server->nosharesock = true;
+ spin_unlock(&server->srv_lock);
+ }
tcon_exit:
free_rsp_buf(resp_buftype, rsp);
base-commit: c0a27675eaf08255017b3cabc28c99c0cd71f468
--
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.