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

[Jeff Layton <jlayton@kernel.org>]

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).

> > +
> >  	/* 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>