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 are 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 /proc/mounts. This mirrors libmount
(util-linux), which replays the current /proc/mounts options on
remount: unless the baseline equals the displayed state, a bare
'mount -o remount' looks like a change and is rejected. Only positive
runtime state is reflected -- no_linux_ext / nopersistent are not
fabricated, as a fabricated value would suppress POSIX / persistent-
handle auto-enable on a later reconnect.
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): cifs_sb->ctx has no lifetime protection.
cifs_show_options() reads string fields (e.g. ctx->iocharset)
locklessly while smb3_reconfigure() frees and replaces them in place,
so a reader racing a remount can take a use-after-free read (KASAN
splat/oops), not merely a stale value. The reconfigure-time dup added
here is one such lockless reader. The exposure is bounded --
reconfigure is privileged and serialized by s_umount -- and the proper
fix (an RCU/refcount ctx lifetime) is a larger, standalone 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.