linux-ext4 - Re: [PATCH] libext2fs: correctly subtract xattr blocks on bigalloc filesystems

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] [day] [month] [year] [list]

Date:   Wed, 24 May 2017 20:10:38 -0700
From:   Eric Biggers <ebiggers3@...il.com>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>,
        Eric Biggers <ebiggers@...gle.com>
Subject: Re: [PATCH] libext2fs: correctly subtract xattr blocks on bigalloc
 filesystems

Hi Andreas,

On Tue, May 23, 2017 at 10:19:35AM -0600, Andreas Dilger wrote:
> On May 21, 2017, at 12:23 AM, Eric Biggers <ebiggers3@...il.com> wrote:
> > 
> > From: Eric Biggers <ebiggers@...gle.com>
> > 
> > ext2fs_inode_data_blocks2() calculates an inode's data block count by
> > subtracting the external xattr block, if any, from the total blocks.
> > But on bigalloc filesystems, the xattr "block" is actually a whole
> > cluster, so ext2fs_inode_data_blocks2() would return a too-large value.
> > 
> > It seems this could have caused several different problems, but the one
> > I encountered was that xfstest generic/399 failed in the "bigalloc"
> > config because e2fsck incorrectly considered a symlink on the filesystem
> > to be corrupted at the end of the test.  This happened because e2fsck
> > incorrectly calculated a nonzero data block count for a "fast" symlink
> > with an external xattr block and therefore treated it as a "slow"
> > symlink, which failed validation.
> 
> I thought we changed this to detect "fast" inodes by i_size < 60 rather
> than using the blocks count, because the blocks count was (and apparently
> continues to be) unreliable for determining fast vs. slow symlinks.
> 
> However, ext4_inode_is_fast_symlink() still checks blocks count.  In
> "[PATCH] ext4: fix reading new encrypted symlinks on no-journal filesystems"
> we discussed whether this was safe, and it appears to be OK from my
> analysis.
> 
> We just continue to hit problems when extrapolating various blocks counts
> to detect fast symlinks rather than just using the same mechanism we use
> at creation time, which is "len > EXT4_N_BLOCKS * 4".
> 
> Cheers, Andreas
> 

Yes, I still think we probably should do that.  This bug needed to be fixed
anyway though, since ext2fs_inode_data_blocks2() is used for a bit more than
just distinguishing between fast and slow symlinks.

Eric