Re: [PATCH] xfs: check for inode size overflow in xfs_new_eof()

[Dave Chinner <david@fromorbit.com>]

On Tue, Sep 30, 2014 at 03:21:23PM -0400, Brian Foster wrote:
> If we write to the maximum file offset (2^63-2), XFS fails to log the
> inode size update when the page is flushed. For example:
> 
> $ xfs_io -fc "pwrite `echo "2^63-1-1" | bc` 1" /mnt/file
> wrote 1/1 bytes at offset 9223372036854775806
> 1.000000 bytes, 1 ops; 0.0000 sec (22.711 KiB/sec and 23255.8140 ops/sec)
> $ stat -c %s /mnt/file
> 9223372036854775807
> $ umount /mnt ; mount <dev> /mnt/
> $ stat -c %s /mnt/file
> 0
> 
> This occurs because XFS calculates the new file size as io_offset +
> io_size, I/O occurs in block sized requests, and the maximum supported
> file size is not block aligned. Therefore, a write to the max allowable
> offset on a 4k blocksize fs results in a write of size 4k to offset
> 2^63-4096 (e.g., equivalent to round_down(2^63-1, 4096), or IOW the
> offset of the block that contains the max file size). The offset plus
> size calculation (2^63 - 4096 + 4096 == 2^63) overflows the signed
> 64-bit variable which goes negative and causes the > comparison to the
> on-disk inode size to fail. This returns 0 from xfs_new_eof() and
> results in no change to the inode on-disk.
> 
> Update xfs_new_eof() to explicitly detect overflow of the local
> calculation and use the VFS inode size in this scenario. The VFS inode
> size is capped to the maximum and thus XFS writes the correct inode size
> to disk.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> 
> Hi all,
> 
> This was also discovered while playing around with xfs/071, though it
> doesn't cause a failure. FWIW, I started off fixing this by converting
> xfs_new_eof() to use xfs_ufsize_t. That worked, but this seemed more
> explicit.
> 
> Brian
> 
>  fs/xfs/xfs_inode.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index c10e3fa..9af2882 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -102,7 +102,7 @@ xfs_new_eof(struct xfs_inode *ip, xfs_fsize_t new_size)
>  {
>  	xfs_fsize_t i_size = i_size_read(VFS_I(ip));
>  
> -	if (new_size > i_size)
> +	if (new_size > i_size || new_size < 0)
>  		new_size = i_size;
>  	return new_size > ip->i_d.di_size ? new_size : 0;
>  }

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs