[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@xxxxxxxxxxxxxxx
Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
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