[PATCH 0/2] smb: client: fix unprivileged-local UAF in cifs_swn_notify

[PATCH 0/2] smb: client: fix unprivileged-local UAF in cifs_swn_notify

[Michael Bommarito]

This series fixes an unprivileged-local use-after-free in the cifs
witness notify path (fs/smb/client/cifs_swn.c +
fs/smb/client/netlink.c).  On any kernel built with
CONFIG_CIFS_SWN_UPCALL=y that has an active witness mount, a local
process of any uid can race cifs_swn_notify() against the reconnect
or umount path and trigger a slab-use-after-free on struct
cifs_tcon.  Patch 1 is the lifetime fix; patch 2 separately closes
the unprivileged-reach surface, because CIFS_GENL_CMD_SWN_NOTIFY
currently admits any uid.  Applies to v7.1-rc2 mainline.

Impact
======

Patch 1 fixes a use-after-free.  cifs_swn_notify() does an
idr_find() under cifs_swnreg_idr_mutex, drops the mutex, and then
dereferences both the returned cifs_swn_reg and swnreg->tcon.
Neither object is pinned across the mutex drop.  A concurrent
cifs_put_tcon() reaching tc_count == 0 calls cifs_swn_unregister()
which frees the swnreg, and tconInfoFree() which frees the tcon
itself.  The race fires both on unmount and from the
smb2_reconnect_server worker on any reconnect cycle.

KASAN under a SMP=4 KVM build with CONFIG_KASAN=y and
kasan.fault=panic_on_write reports a slab-use-after-free in
cifs_swn_notify -> cifs_swn_resource_state_changed (or
cifs_swn_client_move -> cifs_swn_reconnect) on the freed cifs_tcon
under a 400-iteration mount-with-witness/umount loop plus a
4-thread NETLINK_GENERIC notify spammer.  First splat appears in
40-51 seconds on natural timing without any artificial widener.
The freed object is reported as struct cifs_tcon (kmalloc-4k slab),
freed via cifs_put_tcon() from the smb2_reconnect_server worker.

Patch 2 closes the unprivileged surface itself.  The
CIFS_GENL_CMD_SWN_NOTIFY operation in cifs_genl_ops[] currently has
no .flags set, so generic netlink admits it from any uid.  The
intended sender is the cifs.witness userspace helper, which runs
as root via its systemd unit, so requiring CAP_NET_ADMIN in the
initial user namespace does not break any in-tree consumer.

The two patches are orthogonal and both unconditionally beneficial.
Patch 1 fixes the lifetime defect even for an attacker with
CAP_NET_ADMIN; patch 2 prevents reaching the lifetime defect at
all from unprivileged userspace, even before patch 1 lands.

Reach surface
=============

Witness registrations are created when a CIFS client mounts an SMB3
share with the "witness" option against a server advertising
SMB2_SHARE_CAP_CLUSTER on tree-connect (clustered Samba+CTDB,
clustered ONTAP, Windows Scale-Out File Server, Azure SOFS, etc.).
Trigger on the bug is then: any local process able to send a
NETLINK_GENERIC message, raced against the reconnect or umount
path.  Kernels without an active witness mount are not affected.

Validation
==========

On stock the same workload reproduces the splat in 40-51 s of
natural timing from an unprivileged sender (uid 65534).  The
patched kernel runs that workload clean across multiple
multi-minute campaigns:

  - 4 unprivileged-sender instances (200/50/500/100 iter x
    50/500/10/100 ms hold), ~185 s aggregate KASAN-clean.  These
    show patch 2 rejects the send at the genl layer before
    cifs_swn_notify() runs.
  - 4 root-sender instances (1000 iter x 20 ms hold each, the
    densest config tested), ~470 s aggregate KASAN-clean.  These
    deliberately bypass patch 2 and drive cifs_swn_notify()
    directly to validate that patch 1 closes the underlying
    lifetime defect on its own, independent of the genl gate.

A behavioural trace of CIFS_SWN_NOTIFICATION_CLIENT_MOVE confirms
that the cifs.witness daemon still observes the documented wire
sequence after patch 1:

  cifs_swn_client_move -> cifs_swn_reconnect ->
    cifs_swn_unregister -> cifs_swn_send_unregister_message
                           (for old IP, +6.009813s) ->
    cifs_swn_register -> cifs_swn_send_register_message
                         (for new IP, +6.011462s)

Unregister precedes register, matching stock behaviour.  A
narrower fix that also pinned the swnreg across the handler
(rather than only the tcon) would have deferred
cifs_swn_reg_release() past the end of cifs_swn_notify(), and
the daemon would have observed the new-IP register before the
old-IP unregister.  Pinning only the tcon (which is all the
post-mutex code actually dereferences) avoids that ordering trap.

A separate single-process probe confirms patch 2 takes effect:
an unprivileged sendto(NETLINK_GENERIC) of CIFS_GENL_CMD_SWN_NOTIFY
receives -EPERM on the patched kernel and -EINVAL (the cifs handler
ran, no matching registration) on stock.

A reproducer harness (race driver + initramfs glue + QEMU runner)
is available on request.

Patch 1 has a Fixes: tag pointing at fed979a7e082 ("cifs: Set
witness notification handler for messages from userspace daemon");
patch 2 references the same commit since that is where the genl_op
entry was added.

Michael Bommarito (2):
  smb: client: pin tcon across cifs_swn_notify() mutex drop
  smb: client: require GENL_ADMIN_PERM on CIFS_GENL_CMD_SWN_NOTIFY

 fs/smb/client/cifs_swn.c | 64 ++++++++++++++++++++++++++++++++--------
 fs/smb/client/netlink.c  |  1 +
 fs/smb/client/trace.h    |  2 ++
 3 files changed, 54 insertions(+), 13 deletions(-)

--
2.53.0

[PATCH 1/2] smb: client: pin tcon across cifs_swn_notify() mutex drop

[Michael Bommarito]

cifs_swn_notify() looks up a struct cifs_swn_reg by attacker-supplied
registration id with idr_find() under cifs_swnreg_idr_mutex, then drops
the mutex before dereferencing the returned swnreg and swnreg->tcon.
Nothing pins either object across the mutex drop.  A concurrent
cifs_swn_unregister() (reachable from cifs_put_tcon() on the final
tc_count put, which fires both on unmount and from the
smb2_reconnect_server worker on a flaky server) can free the swnreg
before cifs_swn_resource_state_changed() or cifs_swn_client_move()
deref swnreg->tcon, and the tcon backing swnreg has no separate pin
either (cifs_get_swn_reg() stores tcon as a raw pointer with no
matching ++tc_count).

Fix by pinning the tcon under cifs_swnreg_idr_mutex before the drop
and using tcon directly through the rest of the handler.  Take
tc_count on swnreg->tcon under tc_lock, refusing to resurrect a tcon
already marked TID_EXITING (mirrors cifs_find_tcon() discipline).
Refactor cifs_swn_resource_state_changed() and cifs_swn_client_move()
to take struct cifs_tcon * directly; both already only dereferenced
swnreg through ->tcon.

Deliberately do NOT take an extra kref on swnreg here.  The handler
does not dereference swnreg again after the mutex drop, and pinning
it across the rest of the handler would block cifs_swn_unregister()
from running on this id.  That matters because the CLIENT_MOVE path
calls cifs_swn_reconnect() -> cifs_swn_unregister() -> the wire-level
unregister of the old IP, followed by cifs_swn_register() for the new
one.  Holding swnreg's kref would serialise the unregister behind the
notify handler and let the cifs.witness daemon observe register-for-
new-IP before unregister-for-old-IP.

cifs_put_tcon() on tc_count == 0 calls cifs_swn_unregister() which
takes cifs_swnreg_idr_mutex; the put runs outside any swn mutex
section, so there is no recursive acquire.

Lock ordering is consistent with the documented ladder in cifsglob.h
(tc_lock is acquired inside cifs_swnreg_idr_mutex via this and other
swn paths).

Validated against v7.1-rc2 mainline (test build was v7.1-rc2 + a
few commits, SMP=4 KVM, CONFIG_KASAN=y, kasan.fault=panic_on_write).
The same workload that fires the splat naturally on stock (single
splat in 40-51s under a 400-iteration mount/umount + 4-thread notify
spammer loop) ran clean for the duration of multiple multi-minute
campaigns, including campaigns that bypass the CAP_NET_ADMIN gate
added by the companion patch.  A behavioural trace of
CIFS_SWN_NOTIFICATION_CLIENT_MOVE confirms the cifs.witness
unregister-for-old-IP message fires before the register-for-new-IP
message, preserving the existing wire protocol.

Fixes: fed979a7e082 ("cifs: Set witness notification handler for messages from userspace daemon")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
 fs/smb/client/cifs_swn.c | 64 ++++++++++++++++++++++++++++++++--------
 fs/smb/client/trace.h    |  2 ++
 2 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 9753a432d0998..6ebf8a9bb9b6f 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -387,16 +387,16 @@ static void cifs_put_swn_reg(struct cifs_swn_reg *swnreg)
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 }
 
-static int cifs_swn_resource_state_changed(struct cifs_swn_reg *swnreg, const char *name, int state)
+static int cifs_swn_resource_state_changed(struct cifs_tcon *tcon, const char *name, int state)
 {
 	switch (state) {
 	case CIFS_SWN_RESOURCE_STATE_UNAVAILABLE:
 		cifs_dbg(FYI, "%s: resource name '%s' become unavailable\n", __func__, name);
-		cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true);
+		cifs_signal_cifsd_for_reconnect(tcon->ses->server, true);
 		break;
 	case CIFS_SWN_RESOURCE_STATE_AVAILABLE:
 		cifs_dbg(FYI, "%s: resource name '%s' become available\n", __func__, name);
-		cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true);
+		cifs_signal_cifsd_for_reconnect(tcon->ses->server, true);
 		break;
 	case CIFS_SWN_RESOURCE_STATE_UNKNOWN:
 		cifs_dbg(FYI, "%s: resource name '%s' changed to unknown state\n", __func__, name);
@@ -502,7 +502,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
 	return ret;
 }
 
-static int cifs_swn_client_move(struct cifs_swn_reg *swnreg, struct sockaddr_storage *addr)
+static int cifs_swn_client_move(struct cifs_tcon *tcon, struct sockaddr_storage *addr)
 {
 	struct sockaddr_in *ipv4 = (struct sockaddr_in *)addr;
 	struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)addr;
@@ -512,14 +512,16 @@ static int cifs_swn_client_move(struct cifs_swn_reg *swnreg, struct sockaddr_sto
 	else if (addr->ss_family == AF_INET6)
 		cifs_dbg(FYI, "%s: move to %pI6\n", __func__, &ipv6->sin6_addr);
 
-	return cifs_swn_reconnect(swnreg->tcon, addr);
+	return cifs_swn_reconnect(tcon, addr);
 }
 
 int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cifs_swn_reg *swnreg;
+	struct cifs_tcon *tcon = NULL;
 	char name[256];
 	int type;
+	int ret = 0;
 
 	if (info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]) {
 		int swnreg_id;
@@ -527,8 +529,36 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 		swnreg_id = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]);
 		mutex_lock(&cifs_swnreg_idr_mutex);
 		swnreg = idr_find(&cifs_swnreg_idr, swnreg_id);
+		if (swnreg) {
+			/*
+			 * Pin the backing tcon across the mutex drop so a
+			 * concurrent unmount or smb2_reconnect_server worker
+			 * cannot free it before we are done.  Refuse to
+			 * resurrect a tcon already marked TID_EXITING; that
+			 * mirrors cifs_find_tcon() discipline and lets
+			 * teardown make forward progress.
+			 *
+			 * Do NOT take a kref on swnreg here.  The handler
+			 * never dereferences swnreg again after the mutex
+			 * drop; pinning it would block cifs_swn_unregister()
+			 * from running on this id, which is the wire-protocol
+			 * unregister sent by cifs.witness on the CLIENT_MOVE
+			 * reconnect path below.
+			 */
+			spin_lock(&swnreg->tcon->tc_lock);
+			if (swnreg->tcon->status == TID_EXITING) {
+				spin_unlock(&swnreg->tcon->tc_lock);
+			} else {
+				tcon = swnreg->tcon;
+				++tcon->tc_count;
+				trace_smb3_tcon_ref(tcon->debug_id,
+						    tcon->tc_count,
+						    netfs_trace_tcon_ref_get_swn_notify);
+				spin_unlock(&tcon->tc_lock);
+			}
+		}
 		mutex_unlock(&cifs_swnreg_idr_mutex);
-		if (swnreg == NULL) {
+		if (!tcon) {
 			cifs_dbg(FYI, "%s: registration id %d not found\n", __func__, swnreg_id);
 			return -EINVAL;
 		}
@@ -541,7 +571,8 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 		type = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]);
 	} else {
 		cifs_dbg(FYI, "%s: missing notification type attribute\n", __func__);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	switch (type) {
@@ -553,15 +584,18 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 					sizeof(name));
 		} else {
 			cifs_dbg(FYI, "%s: missing resource name attribute\n", __func__);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 		if (info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]) {
 			state = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]);
 		} else {
 			cifs_dbg(FYI, "%s: missing resource state attribute\n", __func__);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
-		return cifs_swn_resource_state_changed(swnreg, name, state);
+		ret = cifs_swn_resource_state_changed(tcon, name, state);
+		break;
 	}
 	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE: {
 		struct sockaddr_storage addr;
@@ -570,16 +604,20 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 			nla_memcpy(&addr, info->attrs[CIFS_GENL_ATTR_SWN_IP], sizeof(addr));
 		} else {
 			cifs_dbg(FYI, "%s: missing IP address attribute\n", __func__);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
-		return cifs_swn_client_move(swnreg, &addr);
+		ret = cifs_swn_client_move(tcon, &addr);
+		break;
 	}
 	default:
 		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__, type);
 		break;
 	}
 
-	return 0;
+out:
+	cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn_notify);
+	return ret;
 }
 
 int cifs_swn_register(struct cifs_tcon *tcon)
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index b99ec5a417fad..5b21ad3c15fb5 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -181,6 +181,7 @@
 	EM(netfs_trace_tcon_ref_get_find,		"GET Find  ") \
 	EM(netfs_trace_tcon_ref_get_find_sess_tcon,	"GET FndSes") \
 	EM(netfs_trace_tcon_ref_get_reconnect_server,	"GET Reconn") \
+	EM(netfs_trace_tcon_ref_get_swn_notify,		"GET SwnNot") \
 	EM(netfs_trace_tcon_ref_new,			"NEW       ") \
 	EM(netfs_trace_tcon_ref_new_ipc,		"NEW Ipc   ") \
 	EM(netfs_trace_tcon_ref_new_reconnect_server,	"NEW Reconn") \
@@ -192,6 +193,7 @@
 	EM(netfs_trace_tcon_ref_put_mnt_ctx,		"PUT MntCtx") \
 	EM(netfs_trace_tcon_ref_put_dfs_refer,		"PUT DfsRfr") \
 	EM(netfs_trace_tcon_ref_put_reconnect_server,	"PUT Reconn") \
+	EM(netfs_trace_tcon_ref_put_swn_notify,		"PUT SwnNot") \
 	EM(netfs_trace_tcon_ref_put_tlink,		"PUT Tlink ") \
 	EM(netfs_trace_tcon_ref_see_cancelled_close,	"SEE Cn-Cls") \
 	EM(netfs_trace_tcon_ref_see_fscache_collision,	"SEE FV-CO!") \
-- 
2.53.0

[PATCH 2/2] smb: client: require GENL_ADMIN_PERM on CIFS_GENL_CMD_SWN_NOTIFY

[Michael Bommarito]

The witness-notify generic-netlink command callback dereferences a
struct cifs_swn_reg looked up by an attacker-supplied registration id,
and the operation entry in cifs_genl_ops[] has no .flags set.
Generic netlink only enforces a capability check when GENL_ADMIN_PERM,
GENL_UNS_ADMIN_PERM, or GENL_OWNER_PERM_CHECK is present on the
operation (net/netlink/genetlink.c).  As written, any local
unprivileged process can send CIFS_SWN_NOTIFICATION_RESOURCE_CHANGE
or CIFS_SWN_NOTIFICATION_CLIENT_MOVE to the in-kernel cifs witness
handler.

The intended sender is the cifs.witness userspace helper, which
runs as root via its systemd unit, so requiring CAP_NET_ADMIN in the
initial user namespace does not break any in-tree consumer.

This patch closes the unprivileged attack surface itself.  The
companion patch ("smb: client: pin tcon across cifs_swn_notify()
mutex drop") closes the lifetime bug exposed by that surface; the
two are independent and both unconditionally beneficial.

Fixes: fed979a7e082 ("cifs: Set witness notification handler for messages from userspace daemon")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
 fs/smb/client/netlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/netlink.c b/fs/smb/client/netlink.c
index 147d9409252cd..acd0cd44a3038 100644
--- a/fs/smb/client/netlink.c
+++ b/fs/smb/client/netlink.c
@@ -33,6 +33,7 @@ static const struct nla_policy cifs_genl_policy[CIFS_GENL_ATTR_MAX + 1] = {
 static const struct genl_ops cifs_genl_ops[] = {
 	{
 		.cmd = CIFS_GENL_CMD_SWN_NOTIFY,
+		.flags = GENL_ADMIN_PERM,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = cifs_swn_notify,
 	},
-- 
2.53.0