Re: [PATCH v5 2/2] xfs: add support for FALLOC_FL_WRITE_ZEROES

[Pankaj Raghav <pankaj.raghav@linux.dev>]

> +
> +	error = xfs_alloc_file_space(ip, offset, len,
> +			XFS_ALLOC_FILE_SPACE_WRITE_ZEROES);
> +	if (error)
> +		return error;
> +
> +	/*
> +	 * xfs_falloc_setsize() would re-zero the written extents via
> +	 * iomap_zero_range(). Use xfs_setfilesize() instead.
> +	 * Update in-core i_size first as xfs_setfilesize() clamps the on-disk
> +	 * size to it.
> +	 */
> +	if (new_size > i_size_read(inode))
> +		i_size_write(inode, new_size);
> +
> +	return xfs_setfilesize(ip, offset, len);

Sashiko reported:
On 32-bit systems where size_t is 32 bits, lengths exceeding 4GB will be
truncated, which might cause the on-disk inode size to be permanently updated
to a severely incorrect smaller size.

So a simple fix would be the following as we already store the value of offset + len locally:

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 37623baaaed6..86fae2190c24 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1426,7 +1426,7 @@ xfs_falloc_write_zeroes(
        if (new_size > i_size_read(inode))
                i_size_write(inode, new_size);

-       return xfs_setfilesize(ip, offset, len);
+       return xfs_setfilesize(ip, new_size, 0);
 }

Probably I will update setfilesize to take 64bit values for len in a separate series.

I will also wait if others have any comments before sending the next version.

@Dave: You were against the initial design[1]. Let me know your thoughts on the current version.

[1] https://lore.kernel.org/linux-xfs/abCzhDSVmFx4PtWI@dread/

--
Pankaj