| From: Eric Biggers <ebiggers@google.com> |
| Date: Thu, 1 Dec 2016 14:57:29 -0500 |
| Subject: ext4: correctly detect when an xattr value has an invalid size |
| |
| commit d7614cc16146e3f0b4c33e71875c19607602aed5 upstream. |
| |
| It was possible for an xattr value to have a very large size, which |
| would then pass validation on 32-bit architectures due to a pointer |
| wraparound. Fix this by validating the size in a way which avoids |
| pointer wraparound. |
| |
| It was also possible that a value's size would fit in the available |
| space but its padded size would not. This would cause an out-of-bounds |
| memory write in ext4_xattr_set_entry when replacing the xattr value. |
| For example, if an xattr value of unpadded size 253 bytes went until the |
| very end of the inode or block, then using setxattr(2) to replace this |
| xattr's value with 256 bytes would cause a write to the 3 bytes past the |
| end of the inode or buffer, and the new xattr value would be incorrectly |
| truncated. Fix this by requiring that the padded size fit in the |
| available space rather than the unpadded size. |
| |
| This patch shouldn't have any noticeable effect on |
| non-corrupted/non-malicious filesystems. |
| |
| Signed-off-by: Eric Biggers <ebiggers@google.com> |
| Signed-off-by: Theodore Ts'o <tytso@mit.edu> |
| [bwh: Backported to 3.16: |
| - s/EFSCORRUPTED/EIO/ |
| - Adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| fs/ext4/xattr.c | 27 +++++++++++++++++++++------ |
| 1 file changed, 21 insertions(+), 6 deletions(-) |
| |
| --- a/fs/ext4/xattr.c |
| +++ b/fs/ext4/xattr.c |
| @@ -188,6 +188,7 @@ ext4_xattr_check_names(struct ext4_xattr |
| { |
| struct ext4_xattr_entry *e = entry; |
| |
| + /* Find the end of the names list */ |
| while (!IS_LAST_ENTRY(e)) { |
| struct ext4_xattr_entry *next = EXT4_XATTR_NEXT(e); |
| if ((void *)next >= end) |
| @@ -195,13 +196,27 @@ ext4_xattr_check_names(struct ext4_xattr |
| e = next; |
| } |
| |
| + /* Check the values */ |
| while (!IS_LAST_ENTRY(entry)) { |
| - if (entry->e_value_size != 0 && |
| - (value_start + le16_to_cpu(entry->e_value_offs) < |
| - (void *)e + sizeof(__u32) || |
| - value_start + le16_to_cpu(entry->e_value_offs) + |
| - le32_to_cpu(entry->e_value_size) > end)) |
| - return -EIO; |
| + if (entry->e_value_size != 0) { |
| + u16 offs = le16_to_cpu(entry->e_value_offs); |
| + u32 size = le32_to_cpu(entry->e_value_size); |
| + void *value; |
| + |
| + /* |
| + * The value cannot overlap the names, and the value |
| + * with padding cannot extend beyond 'end'. Check both |
| + * the padded and unpadded sizes, since the size may |
| + * overflow to 0 when adding padding. |
| + */ |
| + if (offs > end - value_start) |
| + return -EIO; |
| + value = value_start + offs; |
| + if (value < (void *)e + sizeof(u32) || |
| + size > end - value || |
| + EXT4_XATTR_SIZE(size) > end - value) |
| + return -EIO; |
| + } |
| entry = EXT4_XATTR_NEXT(entry); |
| } |
| |