Re: [PATCH v2] udf: Check consistency of Space Bitmap Descriptor

[Jan Kara]

 On Thu 02-02-23 17:04:56, Vladislav Efanov wrote:
> Bits, which are related to Bitmap Descriptor logical blocks,
> are not reset when buffer headers are allocated for them. As the
> result, these logical blocks can be treated as free and
> be used for other blocks.This can cause usage of one buffer header
> for several types of data. UDF issues WARNING in this situation:
> 
> WARNING: CPU: 0 PID: 2703 at fs/udf/inode.c:2014
>   __udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
> 
> RIP: 0010:__udf_add_aext+0x685/0x7d0 fs/udf/inode.c:2014
> Call Trace:
>  udf_setup_indirect_aext+0x573/0x880 fs/udf/inode.c:1980
>  udf_add_aext+0x208/0x2e0 fs/udf/inode.c:2067
>  udf_insert_aext fs/udf/inode.c:2233 [inline]
>  udf_update_extents fs/udf/inode.c:1181 [inline]
>  inode_getblk+0x1981/0x3b70 fs/udf/inode.c:885
> 
> Found by Linux Verification Center (linuxtesting.org) with syzkaller.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Vladislav Efanov <VEfanov@xxxxxxxxx>
> ---
> v2: Do not clear bits related to Bitmap Descriptor logical blocks,
> but return -EFSCORRUPTED error instead.
>  fs/udf/balloc.c  | 24 ++++++++++++++++++++++++
>  fs/udf/udfdecl.h |  1 +
>  2 files changed, 25 insertions(+)

Thanks for the fix!

>  	bitmap->s_block_bitmap[bitmap_nr] = bh;
> +	/* Check consistency of Space Bitmap buffer. */
> +	if (bh) {
> +		max_bits_others = sb->s_blocksize * 8;
> +		max_bits_1st = max_bits_others - (sizeof(struct spaceBitmapDesc) << 3);
> +		rest_bits = (bitmap->s_nr_groups > max_bits_1st) ?
> +					bitmap->s_nr_groups - max_bits_1st : 0;
> +		if (!bitmap_nr)
> +			max_bits = min(max_bits_1st, bitmap->s_nr_groups);
> +		else if (bitmap_nr < rest_bits / max_bits_others + 1)
> +			max_bits = max_bits_others;

So this should be using DIV_ROUND_UP() instead of plain division and + 1
AFAICT. Anyway, I've somewhat simplified these conditions to make things a
bit more obvious and applied your patch. The result is attached for your
reference.

> +		else if (bitmap_nr == rest_bits / max_bits_others + 1)
> +			max_bits = rest_bits % max_bits_others;
> +		for (i = 0; i < max_bits; i++) {
> +			if (udf_test_bit(i + (bitmap_nr ? 0 :
> +				(sizeof(struct spaceBitmapDesc) << 3)),
> +				 bh->b_data))
> +				return -EFSCORRUPTED;
> +		}
> +	}


								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR