Re: [PATCH] nfsd: Use correct error code when decoding extents

[Christoph Hellwig <hch@infradead.org>]

On Wed, Jun 11, 2025 at 12:29:51PM -0400, Chuck Lever wrote:
> On 6/11/25 12:24 PM, Sergey Bashirov wrote:
> > I also have some doubts about this code:
> > if (xdr_stream_decode_u64(&xdr, &bex.len))
> >         return -NFS4ERR_BADXDR;
> > if (bex.len & (block_size - 1))
> >         return -NFS4ERR_BADXDR;
> > 
> > The first error code is clear to me, it is all about decoding. But should
> > not we return -NFS4ERR_EINVAL in the second check? On one hand, we
> > encountered an invalid value after successful decoding, but on the other
> > hand, we stopped decoding the extent array, so we can say that this is
> > also a decoding error.
> 
> On first read of Section 2.3 of RFC 5663, there's no mandated alignment
> requirement for bex_length. IMO this is a case where the implementation
> is deciding that a decoded value is not valid, so NFS4ERR_INVAL might be
> a better choice here.

Section 2.1 of RFC 5663 says:

Clients must be able to perform I/O to the block extents without 
affecting additional areas of storage (especially important for writes);
therefore, extents MUST be aligned to 512-byte boundaries, and writable
extents MUST be aligned to the block size used by the NFSv4 server in
managing the actual file system (4 kilobytes and 8 kilobytes are common
block sizes).  This block size is available as the NFSv4.1 layout_blksize
attribute.

While it would be nice to state this again in 2.3, the language looks
normative enough (TM) to me.