Re: [PATCH v2] SUNRPC: Check if we need to recalculate slack estimates

[Scott Mayhew <smayhew@redhat.com>]

On Thu, 20 Nov 2025, Jeff Layton wrote:

> On Thu, 2025-11-20 at 08:44 -0500, Chuck Lever wrote:
> > On 11/20/25 7:12 AM, Scott Mayhew wrote:
> > > If the incoming GSS verifier is larger than what we previously recorded
> > > on the gss_auth, that would indicate the GSS cred/context used for that
> > > RPC is using a different enctype than the one used by the machine
> > > cred/context, and we should recalculate the slack variables accordingly.
> > > 
> > > Link: https://bugs.debian.org/1120598
> > 
> > Since there is a bug link, a Fixes: tag is recommended.
> > 
> > 
> > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > ---
> > >  net/sunrpc/auth_gss/auth_gss.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> > > index 5c095cb8cb20..bff5f10581a2 100644
> > > --- a/net/sunrpc/auth_gss/auth_gss.c
> > > +++ b/net/sunrpc/auth_gss/auth_gss.c
> > > @@ -1721,6 +1721,18 @@ gss_validate(struct rpc_task *task, struct xdr_stream *xdr)
> > >  	if (maj_stat)
> > >  		goto bad_mic;
> > >  
> > > +	/*
> > > +	 * Normally we only recalculate the slack variables once after
> > > +	 * creating a new gss_auth, but we should also do it if the incoming
> > > +	 * verifier has a larger size than what was previously recorded.
> > > +	 * When the incoming verifier is larger than expected, the
> > > +	 * GSS context is using a different enctype than the one used
> > > +	 * initially by the machine credential. Force a slack size update
> > > +	 * to maintain good payload alignment.
> > > +	 */
> > > +	if (cred->cr_auth->au_verfsize < (XDR_QUADLEN(len) + 2))
> > > +		__set_bit(RPCAUTH_AUTH_UPDATE_SLACK, &cred->cr_auth->au_flags);
> > 
> > set_bit() rather than __set_bit is a better choice for a lockless update
> > where multiple concurrent threads can have access to the flags field.
> > 
> > 
> 
> This function tests for that flag just below though. Could another task
> do the test_and_clear_bit() in gss_update_rslack(), such that the
> au_verfsize doesn't get updated below ?
> 
> If that is a possibility, maybe you should update the au_verfsize
> first, and then the flag just afterward (with a barrier between).

Yes, I suppose that is a possibility.  But in that case we are pretty
much screwed (at least in the case of krb5i and krb5p) because it's
likely that the fields used to generate the 'before' and 'after' values
passed gss_update_rslack() don't coincide with the verifier anyways.

IOW if we're going to recalculate rslack and ralign based on a larger
verifier, then it pretty much has to be done by the same task that sets
the flag in the first place.

> 
> > > +
> > >  	/* We leave it to unwrap to calculate au_rslack. For now we just
> > >  	 * calculate the length of the verifier: */
> > >  	if (test_bit(RPCAUTH_AUTH_UPDATE_SLACK, &cred->cr_auth->au_flags))
> > 
> > Thanks for pursuing this one, Scott.
> > 
> > Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> > 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>
>