Re: [PATCH RFC 1/6] NFSD: Fix NFSv4 SETATTR's handling of large file sizes

[Dave Chinner <david@fromorbit.com>]

On Thu, Jan 27, 2022 at 11:08:31AM -0500, Chuck Lever wrote:
> iattr::ia_size is a loff_t. decode_fattr4() dumps a full u64 value
> in there. If that value is larger than S64_MAX, then ia_size has
> underflowed.
> 
> In this case the negative size is passed through to the VFS and
> underlying filesystems. I've observed XFS behavior: it returns
> EIO but still attempts to access past the end of the device.

What attempts to access beyond the end of the device? A file offset
is not a disk offset, and the filesystem cannot allocate blocks for
IO that are outside the device boundaries. So I don't understand how
setting an inode size of >LLONGMAX can cause the filesystem to
access blocks outside the range it can allocate and map IO to. If
this falls through to trying to access data outside the range the
filesystem is allowed to access then we've got a bug that needs to
be fixed.

Can you please clarify the behaviour that is occurring here (stack
traces demonstrating the IO path that leads to access past the end
of device would be useful) so we can look into this further?

> IOW it assumes the caller has already sanity-checked the value.

Every filesystem assumes that the iattr that is passed to ->setattr
by notify_change() has been sanity checked and the parameters are
within the valid VFS supported ranges, not just XFS. Perhaps this
check should be in notify_change, not in the callers?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com