Re: [PATCH] udf: reject extents past EOF when extending a file

[Jan Kara]

On Mon 09-11-26 09:39:52, xujiahao wrote:
> udf_extend_file() warns when inode_bmap() reports the last extent as
> longer than the block the file is being extended in:
> 
> 	WARN_ON_ONCE(within_last_ext &&
> 		     elen > ((loff_t)offset + 1) << inode->i_blkbits);
> 
> A corrupted image can trigger this, as syzbot reports for
> udf_setsize():
> 
>   WARNING: CPU: 1 PID: 6098 at fs/udf/inode.c:684 udf_extend_file fs/udf/inode.c:683 [inline]
>   WARNING: CPU: 1 PID: 6098 at fs/udf/inode.c:684 udf_setsize+0xd5a/0x1010 fs/udf/inode.c:1300
> 
> The extent really is invalid, so there is nothing to warn about: report
> it with udf_err() and fail the truncate with -EFSCORRUPTED instead of
> tainting the kernel.
> 
> Reported-by: syzbot+db6df8c0f578bc11e50e@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=db6df8c0f578bc11e50e
> Fixes: 16d055656814 ("udf: Discard preallocation before extending file with a hole")
> Signed-off-by: xujiahao <xujiahao@xxxxxxxxxxxxxxxxxxxx>

Thanks! I've added the patch to my tree. I've just slightly modified the
error message to contain inode number.

								Honza


> ---
>  fs/udf/inode.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index bacf2a861..b68aee826 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -687,9 +687,14 @@ static int udf_extend_file(struct inode *inode, loff_t newsize)
>  	if (err < 0)
>  		goto out;
>  	within_last_ext = (err == 1);
> -	/* We don't expect extents past EOF... */
> -	WARN_ON_ONCE(within_last_ext &&
> -		     elen > ((loff_t)offset + 1) << inode->i_blkbits);
> +	if (within_last_ext &&
> +	    elen > ((loff_t)offset + 1) << inode->i_blkbits) {
> +		udf_err(sb, "extent past EOF: length %u, expected at most %lld\n",
> +			elen,
> +			(long long)(((loff_t)offset + 1) << inode->i_blkbits));
> +		err = -EFSCORRUPTED;
> +		goto out;
> +	}
>  
>  	if ((!epos.bh && epos.offset == udf_file_entry_alloc_offset(inode)) ||
>  	    (epos.bh && epos.offset == sizeof(struct allocExtDesc))) {
> -- 
> 2.43.0
> 
> 
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR