linux-kernel - Re: [PATCH] udf: Reserve bits for Bitmap Descriptor buffers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230124084303.pn7glett53qh6pcp@quack3>
Date:   Tue, 24 Jan 2023 09:43:03 +0100
From:   Jan Kara <jack@...e.cz>
To:     Vladislav Efanov <VEfanov@...ras.ru>
Cc:     Jan Kara <jack@...e.com>, linux-kernel@...r.kernel.org,
        lvc-project@...uxtesting.org
Subject: Re: [PATCH] udf: Reserve bits for Bitmap Descriptor buffers

On Fri 20-01-23 12:08:58, 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@...ras.ru>

Thanks for the analysis and the fix. Two notes below:

> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 8e597db4d971..52dab5b63715 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -37,6 +37,7 @@ static int read_block_bitmap(struct super_block *sb,
>  {
>  	struct buffer_head *bh = NULL;
>  	int retval = 0;
> +	int i;
>  	struct kernel_lb_addr loc;
>  
>  	loc.logicalBlockNum = bitmap->s_extPosition;
> @@ -47,6 +48,12 @@ static int read_block_bitmap(struct super_block *sb,
>  		retval = -EIO;
>  
>  	bitmap->s_block_bitmap[bitmap_nr] = bh;
> +	/* Reserve bits for Space Bitmap buffer headers. */
> +	if (bh && !bitmap_nr)
> +		for (i = 0; i < bitmap->s_nr_groups; i++)
> +			udf_clear_bit(bitmap->s_extPosition + i +
> +				      (sizeof(struct spaceBitmapDesc) << 3),
> +				      bh->b_data);
>  	return retval;
>  }

Rather than just siletly making blocks allocated, we should test whether
the block is allocated and if not, return an error (-EFSCORRUPTED) and
refuse to use the filesystem.

Also with 512-byte blocksize one block describes 2MB of the filesystem so
for a filesystem larger than 8GB bitmap->s_nr_groups can be larger than a
number of bits in a block. So we need to be a bit more careful when
verifying the bitmap.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR