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@xxxxxxxxxxxxx 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.
--
Chuck Lever
