Re: [GIT PULL] NFSD changes for v6.18

Trond Myklebust
2025-10-16T15:19:26Z

On Thu, 2025-10-16 at 11:04 -0400, Chuck Lever wrote:
> On 10/16/25 10:36 AM, Geert Uytterhoeven wrote:
> > Hi Jeff,
> >
> > On Thu, 16 Oct 2025 at 16:31, Jeff Layton <jlayton@kernel.org>
> > wrote:
> > > On Mon, 2025-10-13 at 15:37 -0400, Chuck Lever wrote:
> > > > On 10/13/25 3:21 PM, Eric Biggers wrote:
> > > > > On Mon, Oct 13, 2025 at 12:15:52PM +0200, Geert Uytterhoeven
> > > > > wrote:
> > > > > > Hi Chuck, Eric,
> > > > > >
> > > > > > On Wed, 8 Oct 2025 at 00:05, Chuck Lever <cel@kernel.org>
> > > > > > wrote:
> > > > > > > Eric Biggers (4):
> > > > > > >       SUNRPC: Make RPCSEC_GSS_KRB5 select CRYPTO instead
> > > > > > > of depending on it
> > > > > >
> > > > > > This is now commit d8e97cc476e33037 ("SUNRPC: Make
> > > > > > RPCSEC_GSS_KRB5
> > > > > > select CRYPTO instead of depending on it") in v6.18-rc1.
> > > > > > As RPCSEC_GSS_KRB5 defaults to "y", CRYPTO is now auto-
> > > > > > enabled in
> > > > > > defconfigs that didn't enable it before.
> > > > >
> > > > > Now the config is:
> > > > >
> > > > >     config RPCSEC_GSS_KRB5
> > > > >         tristate "Secure RPC: Kerberos V mechanism"
> > > > >         depends on SUNRPC
> > > > >         default y
> > > > >         select SUNRPC_GSS
> > > > >         select CRYPTO
> > > > >         select CRYPTO_SKCIPHER
> > > > >         select CRYPTO_HASH
> > > > >
> > > > > Perhaps the 'default y' should be removed?
> > > > >
> > > > > Chuck, do you know why it's there?
> > > > The "default y" was added by 2010 commit df486a25900f ("NFS:
> > > > Fix the
> > > > selection of security flavours in Kconfig"), then modified
> > > > again by
> > > > commit e3b2854faabd ("SUNRPC: Fix the SUNRPC Kerberos V
> > > > RPCSEC_GSS
> > > > module dependencies") in 2011.
> > > >
> > > > Copying Trond, the author of both of those patches.
> > >
> > > Looking at this a bit closer, maybe a patch like this is what we
> > > want?
> > > This should make it so that we only enable RPCSEC_GSS_KRB5 if
> > > CRYPTO is
> > > already enabled:
> > >
> > > diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
> > > index 984e0cf9bf8a..d433626c7917 100644
> > > --- a/net/sunrpc/Kconfig
> > > +++ b/net/sunrpc/Kconfig
> > > @@ -19,9 +19,8 @@ config SUNRPC_SWAP
> > >  config RPCSEC_GSS_KRB5
> > >         tristate "Secure RPC: Kerberos V mechanism"
> > >         depends on SUNRPC
> > > -       default y
> > > +       default y if CRYPTO
> >
> > This merely controls the default, the user can still override it.
> > Implementing your suggestion above would mean re-adding "depends on
> > CRYPTO", i.e. reverting commit d8e97cc476e33037.
> >
> > >         select SUNRPC_GSS
> > > -       select CRYPTO
> > >         select CRYPTO_SKCIPHER
> > >         select CRYPTO_HASH
> > >         help
> >
> > Gr{oetje,eeting}s,
> >
> >                         Geert
> >
>
> The graph of dependencies and selects between NFS, NFSD, and SUNRPC
> is
> brittle, unfortunately. I suggest reverting d8e97cc476e33037 for now
> while a proper solution is worked out and then tested.
>

Yes. The reason why I went for the weaker 'default y if ...' and
'depends on ...' is precisely because 'select' is so brittle, and at
the time others advised against using it for more complicated
situations such as this. The crypto code has a number of dependencies,
and those have been known to change both over time and across hardware
platforms.

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com