Re: [PATCH v2] ext4: fix BUG_ON() when directory entry has invalid rec_len

[Luís Henriques]

Grr, looks like I accidentally reused a 'git send-email' from shell
history which had a '--in-reply-to' in it.  Please ignore and sorry about
that.  I've just resent a new email.

Cheers,
--
Luís

On Wed, Oct 12, 2022 at 02:13:30PM +0100, Luís Henriques wrote:
> The rec_len field in the directory entry has to be a multiple of 4.  A
> corrupted filesystem image can be used to hit a BUG() in
> ext4_rec_len_to_disk(), called from make_indexed_dir().
> 
>  ------------[ cut here ]------------
>  kernel BUG at fs/ext4/ext4.h:2413!
>  ...
>  RIP: 0010:make_indexed_dir+0x53f/0x5f0
>  ...
>  Call Trace:
>   <TASK>
>   ? add_dirent_to_buf+0x1b2/0x200
>   ext4_add_entry+0x36e/0x480
>   ext4_add_nondir+0x2b/0xc0
>   ext4_create+0x163/0x200
>   path_openat+0x635/0xe90
>   do_filp_open+0xb4/0x160
>   ? __create_object.isra.0+0x1de/0x3b0
>   ? _raw_spin_unlock+0x12/0x30
>   do_sys_openat2+0x91/0x150
>   __x64_sys_open+0x6c/0xa0
>   do_syscall_64+0x3c/0x80
>   entry_SYSCALL_64_after_hwframe+0x46/0xb0
> 
> The fix simply adds a call to ext4_check_dir_entry() to validate the
> directory entry, returning -EFSCORRUPTED if the entry is invalid.
> 
> CC: stable@xxxxxxxxxxxxxxx
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216540
> Signed-off-by: Luís Henriques <lhenriques@xxxxxxx>
> ---
> * Changes since v1:
> 
> As suggested by Ted, I've removed the incorrect 'de->rec_len' check from
> previous version and replaced it with a call to ext4_check_dir_entry()
> instead, which is a much more complete verification.
> 
>  fs/ext4/namei.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 3a31b662f661..ed76e89ffbe9 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2254,8 +2254,16 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
>  	memset(de, 0, len); /* wipe old data */
>  	de = (struct ext4_dir_entry_2 *) data2;
>  	top = data2 + len;
> -	while ((char *)(de2 = ext4_next_entry(de, blocksize)) < top)
> +	while ((char *)(de2 = ext4_next_entry(de, blocksize)) < top) {
> +		if (ext4_check_dir_entry(dir, NULL, de, bh2, data2, len,
> +					 (data2 + (blocksize - csum_size) -
> +					  (char *) de))) {
> +			brelse(bh2);
> +			brelse(bh);
> +			return -EFSCORRUPTED;
> +		}
>  		de = de2;
> +	}
>  	de->rec_len = ext4_rec_len_to_disk(data2 + (blocksize - csum_size) -
>  					   (char *) de, blocksize);
>