linux-ext4 - Re: [PATCH] ext4: limit external inode xattrs to XATTR_SIZE

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: <20180329194125.GC3790@thunk.org>
Date:   Thu, 29 Mar 2018 15:41:25 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     Eric Biggers <ebiggers3@...il.com>,
        linux-ext4 <linux-ext4@...r.kernel.org>,
        Wen Xu <wen.xu@...ech.edu>, Eric Biggers <ebiggers@...gle.com>
Subject: Re: [PATCH] ext4: limit external inode xattrs to XATTR_SIZE_MAX

I tried a variation of this patch (checking against INT_MAX) instead
of XATTR_MAX_SIZE, but it's not sufficient to address the BUG() in

https://bugzilla.kernel.org/show_bug.cgi?id=199185

The reason for that is that initially the xattr block is OK:

[   15.236890] xattr_check_block: 2047 0

but afterwards, due to other file system corruptions (remember, this
is a carefully corrupted file system by malicious attacker), the xattr
block gets corrupted in memory, and then *subsequent* checks of the
block are skipped because the "buffer verified" bit is set:

[   15.239204] EXT4-fs error (device vdc): mb_free_blocks:1456: group 0, inode 16: block 1985:freeing already freed block (bit 1984); block bitmap corrupt.
[   15.243899] EXT4-fs error (device vdc): ext4_mb_generate_buddy:744: group 0, block bitmap and bg descriptor inconsistent: 960 vs 961 free clusters
[   15.248943] xattr_check_block skip: 2047
[   15.251442] xattr_check_block skip: 2047
[   15.252642] ext4_xattr_block_get ERANGE: 2047

Hence, if we don't add a specific check in ext4_xattr_block_get(), we
still return an overflowed size as an error check, and then kernel
will still BUG.

I think it's fair to add an INT_MAX check to
ext4_xattr_check_entries(), but we're still going to need to have
checks in ext4_xattr_block_get() and ext4_xattr_ibody_get().

I agree with Andreas that that the primary check should be INT_MAX,
although I'll note that in actual practice if we want to expand the
XATTR_MAX_SIZE we probably wouldn't want to do that as a ro_compat
feature bit.

Cheers,

					- Ted