Re: [PATCH] ext4: dax: Fix overflowing extents beyond inode size when partially writing

[Jan Kara <jack@suse.cz>]

On Fri 09-08-24 20:15:32, Zhihao Cheng wrote:
> From: Zhihao Cheng <chengzhihao1@huawei.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@vger.kernel.org
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219136
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>

Good catch! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.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@suse.com>
SUSE Labs, CR