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: <0C2D5C52-76C8-401F-8E6F-936834A8DCBE@dilger.ca>
Date:   Fri, 9 Mar 2018 12:12:04 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     Lukas Czerner <lczerner@...hat.com>
Cc:     tytso@....edu, linux-ext4@...r.kernel.org
Subject: Re: [PATCH] e2fsck: fix endianness problem when reading htree nodes

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.

Cheers, Andreas

Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)