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

Posted by Ameer Hamza 1 month, 2 weeks ago

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

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

Posted by Chuck Lever 1 month, 2 weeks ago

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

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

Posted by Jeff Layton 1 month, 2 weeks ago

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

Posted by Ameer Hamza 1 month, 2 weeks ago

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>