[PATCH v2 0/2] sunrpc: don't strip a uid's groups when the mountd group lookup fails

[PATCH v2 0/2] sunrpc: don't strip a uid's groups when the mountd group lookup fails

[Ameer Hamza]

On a server running rpc.mountd with --manage-gids, a transient
name-service failure makes mountd answer the auth.unix.gid upcall
with a zero-group reply, which the kernel installs as a valid
positive entry.  Patch 1 records such replies as negative entries
on the classic /proc channel, so the credential keeps the groups
it already carries.  Patch 2 does the same for the netlink upcall,
which has an explicit NEGATIVE flag the parser does not propagate.

One observation from reviewing the netlink path: in steady-state
netlink-only operation, mountd currently receives no unix_gid
upcalls, because unix_gid_upcall() still gates request generation
on cache_listeners_exist(), which counts only classic-channel
writers, and netlink-mode mountd opens no /proc channels.  A
flagged reply reaches the parser today only via the 30-second
post-close grace window, a concurrent classic-channel writer, or
a direct SUNRPC_CMD_UNIX_GID_SET_REQS.  Patch 2 fixes the parser
semantics ahead of that; making upcall generation netlink-aware
needs a per-cache listener signal (mountd subscribes to the
notification group even without --manage-gids) and is left for a
separate, runtime-tested series.

Patch 1 was tested on a live 6.12 server over NFSv3 and v4.0-v4.2
with sec=sys and sec=krb5, and applies unmodified to every
maintained stable tree, 5.10.y through 7.1.y.  Patch 2 is build-
and static-checked (sparse, W=1) but not runtime-tested.

Changes since v1:
- split the netlink fix into its own patch with its own Fixes:
  tag, as requested by Chuck
- patch 1 is unchanged

v1: https://lore.kernel.org/linux-nfs/20260814172507.1474519-1-ameer.hamza@truenas.com/

Ameer Hamza (2):
  sunrpc: treat empty auth.unix.gid replies as negative entries
  sunrpc: honor the netlink unix_gid NEGATIVE flag

 net/sunrpc/svcauth_unix.c | 9 +++++++++
 1 file changed, 9 insertions(+)


base-commit: 46db3c8a1be96a354758b44b1d4fbb4b70d09a20
-- 
2.53.0

[PATCH v2 1/2] sunrpc: treat empty auth.unix.gid replies as negative entries

[Ameer Hamza]

When rpc.mountd cannot resolve a uid (getpwuid() or getgrouplist()
failure, e.g. while winbind or sssd is briefly unreachable), it
answers the auth.unix.gid upcall with zero groups. unix_gid_parse()
installs that as a valid positive entry, and svcauth_unix_set_client()
then replaces the credential's group list with the empty one on
every request, RPCSEC_GSS included via svcauth_gss_set_client().
One failed lookup strips that uid of all supplementary groups on
every export for up to mountd's configured TTL (30 minutes by
default), long after the NSS backend has recovered.

mountd cannot send an empty list for a successful lookup, since
getgrouplist(3) always includes at least the user's primary group,
so a zero-group reply can only mean the lookup failed. Record it as
a negative entry: unix_gid_find() then returns -ENOENT and
svcauth_unix_set_client() keeps the groups the RPC credential
already carries. This is the fallback that
commit 3fc605a2aa38 ("[PATCH] knfsd: allow the server to provide a
gid list when using AUTH_UNIX authentication") promised when no
answer is available, and the same state try_to_negate_entry()
already creates when no listener holds the channel open.

Fixes: 3fc605a2aa38 ("[PATCH] knfsd: allow the server to provide a gid list when using AUTH_UNIX authentication")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
---
Reproducer (any server with rpc.mountd --manage-gids; 'alice' is a
member of group 'proj' *supplementary*, /srv/proj is root:proj 0750):

    runuser -u alice -- cat /mnt/proj/data          # ok
    # the reply mountd sends when getpwuid()/getgrouplist() fail:
    echo "$(id -u alice) $(( $(date +%s) + 600 )) 0" \
        > /proc/net/rpc/auth.unix.gid/channel
    runuser -u alice -- cat /mnt/proj/data          # EACCES until
                                                    # refresh/flush/expiry

Applies unmodified (fuzz 0) to every maintained stable tree, 5.10.y
through 7.1.y. Tested on a live 6.12-based server over NFSv3 and
v4.0-v4.2, with sec=sys and sec=krb5.

 net/sunrpc/svcauth_unix.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index aebd97e7f66c7..2f592ba54a366 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -540,6 +540,13 @@ static int unix_gid_parse(struct cache_detail *cd,
 	if (ugp) {
 		struct cache_head *ch;
 		ug.h.flags = 0;
+		/*
+		 * mountd sends at least the user's primary group on
+		 * success, so an empty list can only mean the lookup
+		 * failed. Keep the credential's own groups instead.
+		 */
+		if (gids == 0)
+			set_bit(CACHE_NEGATIVE, &ug.h.flags);
 		ug.h.expiry_time = expiry;
 		ch = sunrpc_cache_update(cd,
 					 &ug.h, &ugp->h,
-- 
2.53.0

[PATCH v2 2/2] sunrpc: honor the netlink unix_gid NEGATIVE flag

[Ameer Hamza]

The unix_gid netlink upcall protocol has an explicit
SUNRPC_A_UNIX_GID_NEGATIVE attribute for failed group lookups, and
mountd sends it, but sunrpc_nl_parse_one_unix_gid() only allocates
an empty group list for a flagged reply without propagating the
flag into the entry, so it installs a valid positive entry with
zero groups. Such an entry strips the uid of all supplementary
groups until it is refreshed or expires: the same defect the
previous patch fixes on the classic channel, on a transport that
can say "lookup failed" explicitly.

Set CACHE_NEGATIVE for flagged replies, as the netlink ip_map
path already does for its negative flag. An empty GIDS list
without the flag remains a positive entry.

Fixes: 0850e8603cd7 ("sunrpc: add netlink upcall for the auth.unix.gid cache")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
---
 net/sunrpc/svcauth_unix.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 2f592ba54a366..4dd28749bd5de 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -737,6 +737,8 @@ static int sunrpc_nl_parse_one_unix_gid(struct cache_detail *cd,
 			   boot.tv_sec;
 
 	if (tb[SUNRPC_A_UNIX_GID_NEGATIVE]) {
+		/* failed lookup: keep the credential's own groups */
+		set_bit(CACHE_NEGATIVE, &ug.h.flags);
 		ug.gi = groups_alloc(0);
 		if (!ug.gi)
 			return -ENOMEM;
-- 
2.53.0

Re: [PATCH v2 0/2] sunrpc: don't strip a uid's groups when the mountd group lookup fails

[Jeff Layton]

On Sat, 2026-08-15 at 03:19 +0500, Ameer Hamza wrote:
> On a server running rpc.mountd with --manage-gids, a transient
> name-service failure makes mountd answer the auth.unix.gid upcall
> with a zero-group reply, which the kernel installs as a valid
> positive entry.  Patch 1 records such replies as negative entries
> on the classic /proc channel, so the credential keeps the groups
> it already carries.  Patch 2 does the same for the netlink upcall,
> which has an explicit NEGATIVE flag the parser does not propagate.
> 
> One observation from reviewing the netlink path: in steady-state
> netlink-only operation, mountd currently receives no unix_gid
> upcalls, because unix_gid_upcall() still gates request generation
> on cache_listeners_exist(), which counts only classic-channel
> writers, and netlink-mode mountd opens no /proc channels.  A
> flagged reply reaches the parser today only via the 30-second
> post-close grace window, a concurrent classic-channel writer, or
> a direct SUNRPC_CMD_UNIX_GID_SET_REQS.  Patch 2 fixes the parser
> semantics ahead of that; making upcall generation netlink-aware
> needs a per-cache listener signal (mountd subscribes to the
> notification group even without --manage-gids) and is left for a
> separate, runtime-tested series.
> 

Do you intend to send the separate series for this?

> Patch 1 was tested on a live 6.12 server over NFSv3 and v4.0-v4.2
> with sec=sys and sec=krb5, and applies unmodified to every
> maintained stable tree, 5.10.y through 7.1.y.  Patch 2 is build-
> and static-checked (sparse, W=1) but not runtime-tested.
> 
> Changes since v1:
> - split the netlink fix into its own patch with its own Fixes:
>   tag, as requested by Chuck
> - patch 1 is unchanged
> 
> v1: https://lore.kernel.org/linux-nfs/20260814172507.1474519-1-ameer.hamza@truenas.com/
> 
> Ameer Hamza (2):
>   sunrpc: treat empty auth.unix.gid replies as negative entries
>   sunrpc: honor the netlink unix_gid NEGATIVE flag
> 
>  net/sunrpc/svcauth_unix.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> 
> base-commit: 46db3c8a1be96a354758b44b1d4fbb4b70d09a20

Good catch. Both of these look good to me.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 0/2] sunrpc: don't strip a uid's groups when the mountd group lookup fails

[Ameer Hamza]

On Sat, Aug 15, 2026 at 06:50:48AM -0400, Jeff Layton wrote:
> On Sat, 2026-08-15 at 03:19 +0500, Ameer Hamza wrote:
> > On a server running rpc.mountd with --manage-gids, a transient
> > name-service failure makes mountd answer the auth.unix.gid upcall
> > with a zero-group reply, which the kernel installs as a valid
> > positive entry.  Patch 1 records such replies as negative entries
> > on the classic /proc channel, so the credential keeps the groups
> > it already carries.  Patch 2 does the same for the netlink upcall,
> > which has an explicit NEGATIVE flag the parser does not propagate.
> > 
> > One observation from reviewing the netlink path: in steady-state
> > netlink-only operation, mountd currently receives no unix_gid
> > upcalls, because unix_gid_upcall() still gates request generation
> > on cache_listeners_exist(), which counts only classic-channel
> > writers, and netlink-mode mountd opens no /proc channels.  A
> > flagged reply reaches the parser today only via the 30-second
> > post-close grace window, a concurrent classic-channel writer, or
> > a direct SUNRPC_CMD_UNIX_GID_SET_REQS.  Patch 2 fixes the parser
> > semantics ahead of that; making upcall generation netlink-aware
> > needs a per-cache listener signal (mountd subscribes to the
> > notification group even without --manage-gids) and is left for a
> > separate, runtime-tested series.
> > 
> 
> Do you intend to send the separate series for this?

Not at the moment. I may take a look later if it is still open.

Thanks for the review.

> 
> > Patch 1 was tested on a live 6.12 server over NFSv3 and v4.0-v4.2
> > with sec=sys and sec=krb5, and applies unmodified to every
> > maintained stable tree, 5.10.y through 7.1.y.  Patch 2 is build-
> > and static-checked (sparse, W=1) but not runtime-tested.
> > 
> > Changes since v1:
> > - split the netlink fix into its own patch with its own Fixes:
> >   tag, as requested by Chuck
> > - patch 1 is unchanged
> > 
> > v1: https://lore.kernel.org/linux-nfs/20260814172507.1474519-1-ameer.hamza@truenas.com/
> > 
> > Ameer Hamza (2):
> >   sunrpc: treat empty auth.unix.gid replies as negative entries
> >   sunrpc: honor the netlink unix_gid NEGATIVE flag
> > 
> >  net/sunrpc/svcauth_unix.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > 
> > base-commit: 46db3c8a1be96a354758b44b1d4fbb4b70d09a20
> 
> Good catch. Both of these look good to me.
> -- 
> Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 0/2] sunrpc: don't strip a uid's groups when the mountd group lookup fails

[Chuck Lever]

On Sat, 15 Aug 2026 03:19:51 +0500, Ameer Hamza wrote:
> On a server running rpc.mountd with --manage-gids, a transient
> name-service failure makes mountd answer the auth.unix.gid upcall
> with a zero-group reply, which the kernel installs as a valid
> positive entry.  Patch 1 records such replies as negative entries
> on the classic /proc channel, so the credential keeps the groups
> it already carries.  Patch 2 does the same for the netlink upcall,
> which has an explicit NEGATIVE flag the parser does not propagate.
> 
> [...]

Applied to nfsd-testing, thanks!

[1/2] sunrpc: treat empty auth.unix.gid replies as negative entries
      commit: e865ae50d85a5585eb29cd67dfb0f776da1aa0c9
[2/2] sunrpc: honor the netlink unix_gid NEGATIVE flag
      commit: 76427d869120552a1a82e1f1488d9f8311827d84

--
Chuck Lever