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  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]
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)

Powered by blists - more mailing lists