On Fri, Mar 09, 2018 at 12:12:04PM -0700, Andreas Dilger wrote:
> 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.
Thanks Andreas, I think it makes sense.
-Lukas
>
> Cheers, Andreas
>
>
>
>
>
