[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240809193014.la7uj6rdxfz5p4sl@quack3>
Date: Fri, 9 Aug 2024 21:30:14 +0200
From: Jan Kara <jack@...e.cz>
To: Zhihao Cheng <chengzhihao@...weicloud.com>
Cc: tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
chengzhihao1@...wei.com, yi.zhang@...wei.com
Subject: Re: [PATCH] ext4: dax: Fix overflowing extents beyond inode size
when partially writing
On Fri 09-08-24 20:15:32, Zhihao Cheng wrote:
> From: Zhihao Cheng <chengzhihao1@...wei.com>
>
> The dax_iomap_rw() does two things in each iteration: map written blocks
> and copy user data to blocks. If the process is killed by user(See signal
> handling in dax_iomap_iter()), the copied data will be returned and added
> on inode size, which means that the length of written extents may exceed
> the inode size, then fsck will fail. An example is given as:
>
> dd if=/dev/urandom of=file bs=4M count=1
> dax_iomap_rw
> iomap_iter // round 1
> ext4_iomap_begin
> ext4_iomap_alloc // allocate 0~2M extents(written flag)
> dax_iomap_iter // copy 2M data
> iomap_iter // round 2
> iomap_iter_advance
> iter->pos += iter->processed // iter->pos = 2M
> ext4_iomap_begin
> ext4_iomap_alloc // allocate 2~4M extents(written flag)
> dax_iomap_iter
> fatal_signal_pending
> done = iter->pos - iocb->ki_pos // done = 2M
> ext4_handle_inode_extension
> ext4_update_inode_size // inode size = 2M
>
> fsck reports: Inode 13, i_size is 2097152, should be 4194304. Fix?
>
> Fix the problem by truncating extents if the written length is smaller
> than expected.
>
> Fixes: 776722e85d3b ("ext4: DAX iomap write support")
> CC: stable@...r.kernel.org
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219136
> Signed-off-by: Zhihao Cheng <chengzhihao1@...wei.com>
Good catch! Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> fs/ext4/file.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index c89e434db6b7..be061bb64067 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -334,10 +334,10 @@ static ssize_t ext4_handle_inode_extension(struct inode *inode, loff_t offset,
> * Clean up the inode after DIO or DAX extending write has completed and the
> * inode size has been updated using ext4_handle_inode_extension().
> */
> -static void ext4_inode_extension_cleanup(struct inode *inode, ssize_t count)
> +static void ext4_inode_extension_cleanup(struct inode *inode, bool need_trunc)
> {
> lockdep_assert_held_write(&inode->i_rwsem);
> - if (count < 0) {
> + if (need_trunc) {
> ext4_truncate_failed_write(inode);
> /*
> * If the truncate operation failed early, then the inode may
> @@ -586,7 +586,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
> * writeback of delalloc blocks.
> */
> WARN_ON_ONCE(ret == -EIOCBQUEUED);
> - ext4_inode_extension_cleanup(inode, ret);
> + ext4_inode_extension_cleanup(inode, ret < 0);
> }
>
> out:
> @@ -670,7 +670,7 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
>
> if (extend) {
> ret = ext4_handle_inode_extension(inode, offset, ret);
> - ext4_inode_extension_cleanup(inode, ret);
> + ext4_inode_extension_cleanup(inode, ret < (ssize_t)count);
> }
> out:
> inode_unlock(inode);
> --
> 2.39.2
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists