linux-ext4 - Re: [PATCH] e2fsck: fix endianness problem when reading htree nodes

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: <20180312054257.clzw2x6a7g5yrtcn@rh_laptop>
Date:   Mon, 12 Mar 2018 06:42:57 +0100
From:   Lukas Czerner <lczerner@...hat.com>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     tytso@....edu, linux-ext4@...r.kernel.org
Subject: Re: [PATCH] e2fsck: fix endianness problem when reading htree nodes

On Fri, Mar 09, 2018 at 12:12:04PM -0700, Andreas Dilger wrote:
> On Mar 9, 2018, at 4:28 AM, Lukas Czerner <lczerner@...hat.com> 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@...hat.com>
> 
> 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
> 
> 
> 
> 
>