Re: [RFCv2 1/8] ext2/dax: Fix ext2_setsize when len is page aligned

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Apr 11, 2023 at 10:51:49AM +0530, Ritesh Harjani (IBM) wrote:
> PAGE_ALIGN(x) macro gives the next highest value which is multiple of
> pagesize. But if x is already page aligned then it simply returns x.
> So, if x passed is 0 in dax_zero_range() function, that means the
> length gets passed as 0 to ->iomap_begin().
> 
> In ext2 it then calls ext2_get_blocks -> max_blocks as 0 and hits bug_on
> here in ext2_get_blocks().
> 	BUG_ON(maxblocks == 0);
> 
> Instead we should be calling dax_truncate_page() here which takes
> care of it. i.e. it only calls dax_zero_range if the offset is not
> page/block aligned.
> 
> This can be easily triggered with following on fsdax mounted pmem
> device.
> 
> dd if=/dev/zero of=file count=1 bs=512
> truncate -s 0 file
> 
> [79.525838] EXT2-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
> [79.529376] ext2 filesystem being mounted at /mnt1/test supports timestamps until 2038 (0x7fffffff)
> [93.793207] ------------[ cut here ]------------
> [93.795102] kernel BUG at fs/ext2/inode.c:637!
> [93.796904] invalid opcode: 0000 [#1] PREEMPT SMP PTI
> [93.798659] CPU: 0 PID: 1192 Comm: truncate Not tainted 6.3.0-rc2-xfstests-00056-g131086faa369 #139
> [93.806459] RIP: 0010:ext2_get_blocks.constprop.0+0x524/0x610
> <...>
> [93.835298] Call Trace:
> [93.836253]  <TASK>
> [93.837103]  ? lock_acquire+0xf8/0x110
> [93.838479]  ? d_lookup+0x69/0xd0
> [93.839779]  ext2_iomap_begin+0xa7/0x1c0
> [93.841154]  iomap_iter+0xc7/0x150
> [93.842425]  dax_zero_range+0x6e/0xa0
> [93.843813]  ext2_setsize+0x176/0x1b0
> [93.845164]  ext2_setattr+0x151/0x200
> [93.846467]  notify_change+0x341/0x4e0
> [93.847805]  ? lock_acquire+0xf8/0x110
> [93.849143]  ? do_truncate+0x74/0xe0
> [93.850452]  ? do_truncate+0x84/0xe0
> [93.851739]  do_truncate+0x84/0xe0
> [93.852974]  do_sys_ftruncate+0x2b4/0x2f0
> [93.854404]  do_syscall_64+0x3f/0x90
> [93.855789]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> 
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

Would seem to make sense...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/ext2/inode.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 26f135e7ffce..dc76147e7b07 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1259,9 +1259,8 @@ static int ext2_setsize(struct inode *inode, loff_t newsize)
>  	inode_dio_wait(inode);
>  
>  	if (IS_DAX(inode))
> -		error = dax_zero_range(inode, newsize,
> -				       PAGE_ALIGN(newsize) - newsize, NULL,
> -				       &ext2_iomap_ops);
> +		error = dax_truncate_page(inode, newsize, NULL,
> +					  &ext2_iomap_ops);
>  	else
>  		error = block_truncate_page(inode->i_mapping,
>  				newsize, ext2_get_block);
> -- 
> 2.39.2
>