Re: client can crash nfsd4_encode_fattr4() by setting bit 84

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2025-09-10 at 11:10 -0400, Chuck Lever wrote:
> On 9/10/25 11:08 AM, Jeff Layton wrote:
> > On Wed, 2025-09-10 at 10:53 -0400, Chuck Lever wrote:
> > > On 9/10/25 10:49 AM, Jeff Layton wrote:
> > > > On Wed, 2025-09-10 at 09:55 -0400, rtm@csail.mit.edu wrote:
> > > > > Entry 84 (and a few neighbors) in nfsd4_enc_fattr4_encode_ops[] is
> > > > > NULL, so if a client sets that bit in an OP_VERIFY bitmask, the server
> > > > > will crash here in nfsd_encode_fattr4():
> > > > > 
> > > > >         for_each_set_bit(bit, attr_bitmap,
> > > > >                          ARRAY_SIZE(nfsd4_enc_fattr4_encode_ops)) {
> > > > >                 status = nfsd4_enc_fattr4_encode_ops[bit](xdr, &args);
> > > > 
> > > > Thanks. That looks like a real bug, alright. I think we just need to
> > > > check that nfsd4_enc_fattr4_encode_ops[bit] is non-NULL before calling
> > > > its handler.
> > > > 
> > > > Care to propose a patch?
> > > 
> > > 597 #define FATTR4_WORD2_XATTR_SUPPORT      BIT(FATTR4_XATTR_SUPPORT -
> > > 64)
> > > 598 #define FATTR4_WORD2_TIME_DELEG_ACCESS  BIT(FATTR4_TIME_DELEG_ACCESS
> > > - 64)
> > > 599 #define FATTR4_WORD2_TIME_DELEG_MODIFY  BIT(FATTR4_TIME_DELEG_MODIFY
> > > - 64)
> > > 600 #define FATTR4_WORD2_OPEN_ARGUMENTS     BIT(FATTR4_OPEN_ARGUMENTS -
> > > 64)
> > > 
> > > I think entries for time_deleg_access and time_deleg_modify are missing
> > > in nfsd4_enc_fattr4_encode_ops...
> > > 
> > 
> > Those are typically requested in CB_GETATTR calls. I'm not sure those
> > are legit to request in a GETATTR. Are they?
> > 
> 
> I think GETATTR needs to skip those, and not return an error. So they
> need to be defined as noops, probably.

Nod. That looks like the best fix.
-- 
Jeff Layton <jlayton@kernel.org>