| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240827160703.GU865349@frogsfrogsfrogs>
Date: Tue, 27 Aug 2024 09:07:03 -0700
From: "Darrick J. Wong" <djwong@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: Christian Brauner <brauner@...nel.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Chandan Babu R <chandan.babu@...cle.com>,
Brian Foster <bfoster@...hat.com>, Jens Axboe <axboe@...nel.dk>,
Jan Kara <jack@...e.cz>, Theodore Ts'o <tytso@....edu>,
linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-xfs@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 6/6] xfs: refactor xfs_file_fallocate
On Tue, Aug 27, 2024 at 08:50:50AM +0200, Christoph Hellwig wrote:
> Refactor xfs_file_fallocate into separate helpers for each mode,
> two factors for i_size handling and a single switch statement over the
> supported modes.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
Much less complicated now! :)
Reviewed-by: Darrick J. Wong <djwong@...nel.org>
--D
> ---
> fs/xfs/xfs_file.c | 330 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 208 insertions(+), 122 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 489bc1b173c268..f6e4912769a0d5 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -852,6 +852,192 @@ static inline bool xfs_file_sync_writes(struct file *filp)
> return false;
> }
>
> +static int
> +xfs_falloc_newsize(
> + struct file *file,
> + int mode,
> + loff_t offset,
> + loff_t len,
> + loff_t *new_size)
> +{
> + struct inode *inode = file_inode(file);
> +
> + if ((mode & FALLOC_FL_KEEP_SIZE) || offset + len <= i_size_read(inode))
> + return 0;
> + *new_size = offset + len;
> + return inode_newsize_ok(inode, *new_size);
> +}
> +
> +static int
> +xfs_falloc_setsize(
> + struct file *file,
> + loff_t new_size)
> +{
> + struct iattr iattr = {
> + .ia_valid = ATTR_SIZE,
> + .ia_size = new_size,
> + };
> +
> + if (!new_size)
> + return 0;
> + return xfs_vn_setattr_size(file_mnt_idmap(file), file_dentry(file),
> + &iattr);
> +}
> +
> +static int
> +xfs_falloc_collapse_range(
> + struct file *file,
> + loff_t offset,
> + loff_t len)
> +{
> + struct inode *inode = file_inode(file);
> + loff_t new_size = i_size_read(inode) - len;
> + int error;
> +
> + if (!xfs_is_falloc_aligned(XFS_I(inode), offset, len))
> + return -EINVAL;
> +
> + /*
> + * There is no need to overlap collapse range with EOF, in which case it
> + * is effectively a truncate operation
> + */
> + if (offset + len >= i_size_read(inode))
> + return -EINVAL;
> +
> + error = xfs_collapse_file_space(XFS_I(inode), offset, len);
> + if (error)
> + return error;
> + return xfs_falloc_setsize(file, new_size);
> +}
> +
> +static int
> +xfs_falloc_insert_range(
> + struct file *file,
> + loff_t offset,
> + loff_t len)
> +{
> + struct inode *inode = file_inode(file);
> + loff_t isize = i_size_read(inode);
> + int error;
> +
> + if (!xfs_is_falloc_aligned(XFS_I(inode), offset, len))
> + return -EINVAL;
> +
> + /*
> + * New inode size must not exceed ->s_maxbytes, accounting for
> + * possible signed overflow.
> + */
> + if (inode->i_sb->s_maxbytes - isize < len)
> + return -EFBIG;
> +
> + /* Offset should be less than i_size */
> + if (offset >= isize)
> + return -EINVAL;
> +
> + error = xfs_falloc_setsize(file, isize + len);
> + if (error)
> + return error;
> +
> + /*
> + * Perform hole insertion now that the file size has been updated so
> + * that if we crash during the operation we don't leave shifted extents
> + * past EOF and hence losing access to the data that is contained within
"...and hence lose access to the data..."
> + * them.
> + */
> + return xfs_insert_file_space(XFS_I(inode), offset, len);
> +}
> +
> +/*
> + * Punch a hole and prealloc the range. We use a hole punch rather than
> + * unwritten extent conversion for two reasons:
> + *
> + * 1.) Hole punch handles partial block zeroing for us.
> + * 2.) If prealloc returns ENOSPC, the file range is still zero-valued by
> + * virtue of the hole punch.
> + */
> +static int
> +xfs_falloc_zero_range(
> + struct file *file,
> + int mode,
> + loff_t offset,
> + loff_t len)
> +{
> + struct inode *inode = file_inode(file);
> + unsigned int blksize = i_blocksize(inode);
> + loff_t new_size = 0;
> + int error;
> +
> + trace_xfs_zero_file_space(XFS_I(inode));
> +
> + error = xfs_falloc_newsize(file, mode, offset, len, &new_size);
> + if (error)
> + return error;
> +
> + error = xfs_free_file_space(XFS_I(inode), offset, len);
> + if (error)
> + return error;
> +
> + len = round_up(offset + len, blksize) - round_down(offset, blksize);
> + offset = round_down(offset, blksize);
> + error = xfs_alloc_file_space(XFS_I(inode), offset, len);
> + if (error)
> + return error;
> + return xfs_falloc_setsize(file, new_size);
> +}
> +
> +static int
> +xfs_falloc_unshare_range(
> + struct file *file,
> + int mode,
> + loff_t offset,
> + loff_t len)
> +{
> + struct inode *inode = file_inode(file);
> + loff_t new_size = 0;
> + int error;
> +
> + error = xfs_falloc_newsize(file, mode, offset, len, &new_size);
> + if (error)
> + return error;
> +
> + error = xfs_reflink_unshare(XFS_I(inode), offset, len);
> + if (error)
> + return error;
> +
> + error = xfs_alloc_file_space(XFS_I(inode), offset, len);
> + if (error)
> + return error;
> + return xfs_falloc_setsize(file, new_size);
> +}
> +
> +static int
> +xfs_falloc_allocate_range(
> + struct file *file,
> + int mode,
> + loff_t offset,
> + loff_t len)
> +{
> + struct inode *inode = file_inode(file);
> + loff_t new_size = 0;
> + int error;
> +
> + /*
> + * If always_cow mode we can't use preallocations and thus should not
> + * create them.
> + */
> + if (xfs_is_always_cow_inode(XFS_I(inode)))
> + return -EOPNOTSUPP;
> +
> + error = xfs_falloc_newsize(file, mode, offset, len, &new_size);
> + if (error)
> + return error;
> +
> + error = xfs_alloc_file_space(XFS_I(inode), offset, len);
> + if (error)
> + return error;
> + return xfs_falloc_setsize(file, new_size);
> +}
> +
> #define XFS_FALLOC_FL_SUPPORTED \
> (FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | \
> FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE | \
> @@ -868,8 +1054,6 @@ xfs_file_fallocate(
> struct xfs_inode *ip = XFS_I(inode);
> long error;
> uint iolock = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
> - loff_t new_size = 0;
> - bool do_file_insert = false;
>
> if (!S_ISREG(inode->i_mode))
> return -EINVAL;
> @@ -894,129 +1078,31 @@ xfs_file_fallocate(
> if (error)
> goto out_unlock;
>
> - if (mode & FALLOC_FL_PUNCH_HOLE) {
> + switch (mode & FALLOC_FL_MODE_MASK) {
> + case FALLOC_FL_PUNCH_HOLE:
> error = xfs_free_file_space(ip, offset, len);
> - if (error)
> - goto out_unlock;
> - } else if (mode & FALLOC_FL_COLLAPSE_RANGE) {
> - if (!xfs_is_falloc_aligned(ip, offset, len)) {
> - error = -EINVAL;
> - goto out_unlock;
> - }
> -
> - /*
> - * There is no need to overlap collapse range with EOF,
> - * in which case it is effectively a truncate operation
> - */
> - if (offset + len >= i_size_read(inode)) {
> - error = -EINVAL;
> - goto out_unlock;
> - }
> -
> - new_size = i_size_read(inode) - len;
> -
> - error = xfs_collapse_file_space(ip, offset, len);
> - if (error)
> - goto out_unlock;
> - } else if (mode & FALLOC_FL_INSERT_RANGE) {
> - loff_t isize = i_size_read(inode);
> -
> - if (!xfs_is_falloc_aligned(ip, offset, len)) {
> - error = -EINVAL;
> - goto out_unlock;
> - }
> -
> - /*
> - * New inode size must not exceed ->s_maxbytes, accounting for
> - * possible signed overflow.
> - */
> - if (inode->i_sb->s_maxbytes - isize < len) {
> - error = -EFBIG;
> - goto out_unlock;
> - }
> - new_size = isize + len;
> -
> - /* Offset should be less than i_size */
> - if (offset >= isize) {
> - error = -EINVAL;
> - goto out_unlock;
> - }
> - do_file_insert = true;
> - } else {
> - if (!(mode & FALLOC_FL_KEEP_SIZE) &&
> - offset + len > i_size_read(inode)) {
> - new_size = offset + len;
> - error = inode_newsize_ok(inode, new_size);
> - if (error)
> - goto out_unlock;
> - }
> -
> - if (mode & FALLOC_FL_ZERO_RANGE) {
> - /*
> - * Punch a hole and prealloc the range. We use a hole
> - * punch rather than unwritten extent conversion for two
> - * reasons:
> - *
> - * 1.) Hole punch handles partial block zeroing for us.
> - * 2.) If prealloc returns ENOSPC, the file range is
> - * still zero-valued by virtue of the hole punch.
> - */
> - unsigned int blksize = i_blocksize(inode);
> -
> - trace_xfs_zero_file_space(ip);
> -
> - error = xfs_free_file_space(ip, offset, len);
> - if (error)
> - goto out_unlock;
> -
> - len = round_up(offset + len, blksize) -
> - round_down(offset, blksize);
> - offset = round_down(offset, blksize);
> - } else if (mode & FALLOC_FL_UNSHARE_RANGE) {
> - error = xfs_reflink_unshare(ip, offset, len);
> - if (error)
> - goto out_unlock;
> - } else {
> - /*
> - * If always_cow mode we can't use preallocations and
> - * thus should not create them.
> - */
> - if (xfs_is_always_cow_inode(ip)) {
> - error = -EOPNOTSUPP;
> - goto out_unlock;
> - }
> - }
> -
> - error = xfs_alloc_file_space(ip, offset, len);
> - if (error)
> - goto out_unlock;
> - }
> -
> - /* Change file size if needed */
> - if (new_size) {
> - struct iattr iattr;
> -
> - iattr.ia_valid = ATTR_SIZE;
> - iattr.ia_size = new_size;
> - error = xfs_vn_setattr_size(file_mnt_idmap(file),
> - file_dentry(file), &iattr);
> - if (error)
> - goto out_unlock;
> - }
> -
> - /*
> - * Perform hole insertion now that the file size has been
> - * updated so that if we crash during the operation we don't
> - * leave shifted extents past EOF and hence losing access to
> - * the data that is contained within them.
> - */
> - if (do_file_insert) {
> - error = xfs_insert_file_space(ip, offset, len);
> - if (error)
> - goto out_unlock;
> + break;
> + case FALLOC_FL_COLLAPSE_RANGE:
> + error = xfs_falloc_collapse_range(file, offset, len);
> + break;
> + case FALLOC_FL_INSERT_RANGE:
> + error = xfs_falloc_insert_range(file, offset, len);
> + break;
> + case FALLOC_FL_ZERO_RANGE:
> + error = xfs_falloc_zero_range(file, mode, offset, len);
> + break;
> + case FALLOC_FL_UNSHARE_RANGE:
> + error = xfs_falloc_unshare_range(file, mode, offset, len);
> + break;
> + case FALLOC_FL_ALLOCATE_RANGE:
> + error = xfs_falloc_allocate_range(file, mode, offset, len);
> + break;
> + default:
> + error = -EOPNOTSUPP;
> + break;
> }
>
> - if (xfs_file_sync_writes(file))
> + if (!error && xfs_file_sync_writes(file))
> error = xfs_log_force_inode(ip);
>
> out_unlock:
> --
> 2.43.0
>
>
Powered by blists - more mailing lists