linux-ext4 - Re: [PATCH 1/3] ext4: do not allow external inodes for inline data

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]

Date:   Thu, 24 May 2018 10:51:21 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Theodore Ts'o <tytso@....edu>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>,
        wen.xu@...ech.edu, jannh@...gle.com
Subject: Re: [PATCH 1/3] ext4: do not allow external inodes for inline data

On May 23, 2018, at 9:37 AM, Theodore Ts'o <tytso@....edu> wrote:
> 
> The inline data feature was implemented before we added support for
> external inodes for xattrs.  It makes no sense to support that
> combination, but the problem is that there are a number of extended
> attribute checks that are skipped if e_value_inum is non-zero.
> 
> Unfortunately, the inline data code is completely e_value_inum
> unaware, and attempts to interpret the xattr fields as if it were an
> inline xattr --- at which point, Hilarty Ensues.

I guess the separate question is whether this should also check for
"inline data" in an external xattr block and disallow that also?
On the one hand, "inline data" in an external block doesn't make
sense - why not just store the data in a block directly?

On the other hand, this might be useful at some point in the future
if we have large bigalloc chunks that makes using a whole chunk for
a small file inefficient and we could pack the data from multiple
small files into a single bigalloc chunk as inline data xattrs (maybe
using the inode number as xattr name to keep the xattrs unique)?

Cheers, Andreas

> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199803
> 
> Reported-by: Jann Horn <jannh@...gle.com>
> Signed-off-by: Theodore Ts'o <tytso@....edu>
> Fixes: e50e5129f384 ("ext4: xattr-in-inode support")
> Cc: stable@...nel.org
> ---
> fs/ext4/inline.c | 6 ++++++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 70cf4c7b268a..44b4fcdc3755 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -144,6 +144,12 @@ int ext4_find_inline_data_nolock(struct inode *inode)
> 		goto out;
> 
> 	if (!is.s.not_found) {
> +		if (is.s.here->e_value_inum) {
> +			EXT4_ERROR_INODE(inode, "inline data xattr refers "
> +					 "to an external xattr inode");
> +			error = -EFSCORRUPTED;
> +			goto out;
> +		}
> 		EXT4_I(inode)->i_inline_off = (u16)((void *)is.s.here -
> 					(void *)ext4_raw_inode(&is.iloc));
> 		EXT4_I(inode)->i_inline_size = EXT4_MIN_INLINE_DATA_SIZE +
> --
> 2.16.1.72.g5be1f00a9a
> 


Cheers, Andreas






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