On 6/16/25 8:38 AM, Christoph Hellwig wrote: > 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. No argument from me. Passing in a non-aligned bex_length still doesn't seem to me to be an XDR issue. Failing with NFS4ERR_INVAL seems like the correct server response for this situation. -- Chuck Lever
