Re: [PATCH v2 0/2] smb: client: fix CIFS SWN notify lifetime and permissions

[Steve French]

tentatively merged into cifs-2.6.git for-next pending more review and testing

On Sun, May 17, 2026 at 7:12 PM Michael Bommarito
<michael.bommarito@xxxxxxxxx> wrote:
>
> This is v2 of the CIFS witness notify fix series.  v1 fixed the
> basic cifs_swn_notify() use-after-free and added GENL_ADMIN_PERM to
> the incoming notify command, but review pointed out that the lifetime
> fix still trusted the raw tcon pointer cached in cifs_swn_reg.
>
> That cache is unsafe because cifs_get_swn_reg() lets multiple tcons
> for the same net/share name share one witness registration id.  If the
> first tcon goes away while another same-share tcon keeps the
> registration alive, swnreg->tcon can dangle.  Taking tc_lock through
> that pointer is therefore still a use-after-free, and taking tc_lock
> while holding cifs_swnreg_idr_mutex also violates the documented CIFS
> lock order.
>
> Patch 1 changes the SWN registration model so the registration stores
> only stable witness identity: registration id, net name, share name,
> and notify flags.  Notify handling copies that identity under
> cifs_swnreg_idr_mutex, drops the mutex, and then finds and pins a live
> matching tcon under the normal cifs_tcp_ses_lock -> tc_lock order.
> Register and unregister messages use the caller's live tcon rather
> than a cached registration tcon, and the unregister path no longer
> finds a registration, drops the mutex, and later puts a raw pointer.
>
> The intended one-registration/many-tcon semantics are therefore:
> a registration id represents a net/share pair, and notify handling acts
> on a live representative selected at use time.  If the registration id
> exists but no live matching tcon remains, cifs_swn_notify() reports
> that separately instead of logging "registration id not found".
>
> Patch 2 keeps the GENL_ADMIN_PERM gate for SWN_NOTIFY and also adds
> GENL_MCAST_CAP_NET_ADMIN to CIFS_GENL_MCGRP_SWN.  The multicast group
> carries register messages that include the registration id and, for
> NTLM-authenticated mounts, username/domain/password attributes copied
> from the CIFS session, so unprivileged local users should not be able
> to join the group.
>
> Build, static, and runtime validation for this revision:
>
> Targeted UM build of fs/smb/client/cifs_swn.o and fs/smb/client/netlink.o
> on top of v7.1-rc2 rebuilt both touched objects with no new warnings.
> scripts/checkpatch.pl --strict on both patches is clean.
>
> I also ran a KASAN + PROVE_LOCKING QEMU build with the existing ksmbd
> test harness that advertises CLUSTER capability so the client witness
> path is exercised:
>
>   - root-sender race campaign, four parallel mount/umount profiles,
>     using root notify senders to bypass GENL_ADMIN_PERM and stress the
>     lifetime fix directly: no KASAN, oops, or lockdep signatures
>   - same-share regression: two witness mounts with nosharesock shared
>     one registration id; after unmounting the first tcon, CLIENT_MOVE
>     against that id completed successfully on the remaining live tcon
>   - CLIENT_MOVE trace: unregister-for-old-IP still precedes
>     register-for-new-IP
>   - echo/check path: echo_interval=1 drove cifs_swn_check() while
>     DebugData exercised cifs_swn_dump()
>   - SWN_NOTIFY permission probe: uid 65534 gets -EPERM; root reaches
>     the handler and receives the expected no-registration -EINVAL
>   - multicast permission probe: uid 65534 gets -EPERM joining
>     CIFS_GENL_MCGRP_SWN; root joins successfully
>
> The notable runtime results are summarized above.
>
> Changes since v1:
>
>   - remove the raw struct cifs_tcon pointer from struct cifs_swn_reg
>   - resolve and pin a live matching tcon after dropping the SWN idr
>     mutex
>   - avoid taking tc_lock while holding cifs_swnreg_idr_mutex
>   - keep unregister send and kref put under one SWN mutex section
>   - distinguish "registration id not found" from "no live tcon"
>   - mirror extract_hostname() / extract_sharename() byte-for-byte in
>     the new cifs_swn_tcon_matches() helper to avoid GFP_KERNEL
>     allocations under cifs_tcp_ses_lock and tcon->tc_lock
>   - restrict joins to the CIFS SWN multicast group
>   - add runtime coverage for the shared-registration case called out
>     in v1 review
>
> Michael Bommarito (2):
>   smb: client: resolve SWN tcon from live registrations
>   smb: client: require net admin for CIFS SWN netlink
>
>  fs/smb/client/cifs_swn.c | 314 +++++++++++++++++++++++++++++++--------
>  fs/smb/client/netlink.c  |   6 +-
>  fs/smb/client/trace.h    |   2 +
>  3 files changed, 267 insertions(+), 55 deletions(-)
>
> --
> 2.53.0
>


-- 
Thanks,

Steve