On Mar 9, 2018, at 4:28 AM, Lukas Czerner <lczerner@xxxxxxxxxx> wrote:
>
> Wrong directory block number can be saved in ->previous on big endian
> system in parse_int_node(). Fix it by moving the mask out of the endian
> conversion.
>
> Fixes: ae9efd05a986 ("e2fsck: 3 level hash tree directory optimization")
> Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>
I was just going to make a comment about there being a cosmetic issue that
it should use 0x00ffffff to make it clear the whole top byte was masked
off, and/or add a #define to indicate what this value actually is, but
looking at this more closely it seems there is also another bug here...
> diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
> index 1b0504c..345b29e 100644
> --- a/e2fsck/pass2.c
> +++ b/e2fsck/pass2.c
> @@ -664,7 +664,7 @@ static void parse_int_node(ext2_filsys fs,
> }
>
> dx_db->previous =
> - i ? ext2fs_le32_to_cpu(ent[i-1].block & 0x0ffffff) : 0;
> + i ? ext2fs_le32_to_cpu(ent[i-1].block) & 0x0ffffff : 0;
The dx_get_block() function in the kernel uses "0x0fffffff" (28 bits) to mask
the logical block number, so the directory size can be up to 2^28*blocksize
(=1TB for 4KB blocks) vs. previous limit of 2^24*blocksize (=64GB). The old
limit was too small for a 3-level htree that allows up to 2^27 leaf blocks
on 4KB block filesystems (blocksize/8 ^ 3).
We haven't yet implemented the "use top bits to store block fullness to help
leaf block compaction" feature, so expanding the mask doesn't hurt us.
Rather than using 0xffffff directly, it makes sense to add something like:
#define EXT4_DX_BLOCK_MASK 0x0fffffff
and use this in both the kernel and e2fsprogs? I checked the rest of e2fsprogs
and it looks like this is the only place where this mask is used.
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
