Re: [PATCH] ext4: do not accept rec_len 0 for block sizes below 64k

[Jan Kara]

On Tue 06-10-26 07:15:46, hengyul@xxxxxxxxxx wrote:
> From: Hengyu Liang <hengyul@xxxxxxxxxx>
> 
> Commit afa6d5a16bf2 ("ext4: remove PAGE_SIZE checks for rec_len
> conversion") made ext4_rec_len_from_disk() read a rec_len of 0 as "this
> entry covers the whole block" for every block size.
> 
> However, this special value only exists for block sizes of 64k and
> more. With smaller blocks, a rec_len of 0 means that the directory
> block is corrupted. As of now, a directory block that has been
> overwritten with zeroes is treated as an empty block. The kernel no
> longer reports the corruption and can store new entries in that block,
> while e2fsck still reports the block as corrupted.
> 
> The issue can be reproduced on a file system without metadata_csum:
> 
>     mke2fs -q -t ext4 -b 1024 -O ^metadata_csum,^dir_index img 8M
>     mount -o loop img /mnt
>     mkdir /mnt/d
>     for i in $(seq 100); do touch /mnt/d/file_with_a_long_name_$i; done
>     umount /mnt
>     dd if=/dev/zero of=img bs=1024 count=1 conv=notrunc \
>         seek=$(debugfs -R "bmap d 1" img)
>     mount -o loop img /mnt
>     touch /mnt/d/new
> 
> Before commit afa6d5a16bf2 ("ext4: remove PAGE_SIZE checks for rec_len
> conversion"), the last command fails with "Structure needs cleaning".
> After that commit, it succeeds.
> 
> This patch makes ext4_rec_len_from_disk() return the on-disk value
> unchanged when the block size is below 64k, as e2fsprogs does.
> 
> Fixes: afa6d5a16bf2 ("ext4: remove PAGE_SIZE checks for rec_len conversion")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>

Makes sense. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

BTW, the kernel never allowed larger than 64k block size so I'm kind of at
loss why we have that strange code trying to accommodate upto 256k
blocksize here. I'd just delete that code (not really related to your
chnage). Ted?

								Honza

> ---
>  fs/ext4/ext4.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 724a27e8be61..ab716a5ad6da 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2611,6 +2611,13 @@ ext4_rec_len_from_disk(__le16 dlen, unsigned blocksize)
>  {
>  	unsigned len = le16_to_cpu(dlen);
>  
> +	/*
> +	 * Only blocks of 64k and more need the special encoding of rec_len.
> +	 * For smaller blocks 0 and EXT4_MAX_REC_LEN are not valid lengths
> +	 * and must not be taken for an entry that spans the whole block.
> +	 */
> +	if (blocksize < 65536)
> +		return len;
>  	if (len == EXT4_MAX_REC_LEN || len == 0)
>  		return blocksize;
>  	return (len & 65532) | ((len & 3) << 16);
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR