I've done the following which should hopefully make you happier and things clearer. First of all, I've taken the extra checks and moved out of this commit. So it now looks pretty much like your original proposed patch. Then I've added two separate patches to add better bounds checking to the xattr read and find path. There is almost certainly more paranoia checks that could be added later --- in particular in the xattr set codepaths --- but this is the low-hanging fruit to make life more interesting for people doing research in file system fuzzing tools. :-) - Ted >From ce3fd194fcc6fbdc00ce095a852f22df97baa401 Mon Sep 17 00:00:00 2001 From: Eric Biggers <ebiggers@xxxxxxxxxx> Date: Thu, 29 Mar 2018 14:31:42 -0400 Subject: [PATCH] ext4: limit xattr size to INT_MAX ext4 isn't validating the sizes of xattrs where the value of the xattr is stored in an external inode. This is problematic because ->e_value_size is a u32, but ext4_xattr_get() returns an int. A very large size is misinterpreted as an error code, which ext4_get_acl() translates into a bogus ERR_PTR() for which IS_ERR() returns false, causing a crash. Fix this by validating that all xattrs are <= INT_MAX bytes. This issue has been assigned CVE-2018-1095. https://bugzilla.kernel.org/show_bug.cgi?id=199185 https://bugzilla.redhat.com/show_bug.cgi?id=1560793 Reported-by: Wen Xu <wen.xu@xxxxxxxxxx> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx> Cc: stable@xxxxxxxxxxxxxxx Fixes: e50e5129f384 ("ext4: xattr-in-inode support") --- fs/ext4/xattr.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 63656dbafdc4..2077d87b09f2 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -195,10 +195,13 @@ ext4_xattr_check_entries(struct ext4_xattr_entry *entry, void *end, /* Check the values */ while (!IS_LAST_ENTRY(entry)) { - if (entry->e_value_size != 0 && - entry->e_value_inum == 0) { + u32 size = le32_to_cpu(entry->e_value_size); + + if (size > INT_MAX) + return -EFSCORRUPTED; + + if (size != 0 && entry->e_value_inum == 0) { u16 offs = le16_to_cpu(entry->e_value_offs); - u32 size = le32_to_cpu(entry->e_value_size); void *value; /* -- 2.16.1.72.g5be1f00a9a
