ext4: add extra checks to ext4_xattr_block_get()

BugLink: http://bugs.launchpad.net/bugs/1773233
commit 54dd0e0a1b255f115f8647fc6fb93273251b01b9 upstream.

Add explicit checks in ext4_xattr_block_get() just in case the
e_value_offs and e_value_size fields in the the xattr block are
corrupted in memory after the buffer_verified bit is set on the xattr
block.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Acked-by: Khalid Elmously <khalid.elmously@canonical.com>
Acked-by: Brad Figg <brad.figg@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 88544d6f2cb3a9a25492234e5e545aa9fdbddf84..1718354e6322e524f58a83d3d317fc4876973fb0 100644 (file)
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -196,7 +196,7 @@ ext4_xattr_check_entries(struct ext4_xattr_entry *entry, void *end,
        while (!IS_LAST_ENTRY(entry)) {
                u32 size = le32_to_cpu(entry->e_value_size);
 
-               if (size > INT_MAX)
+               if (size > EXT4_XATTR_SIZE_MAX)
                        return -EFSCORRUPTED;
 
                if (size != 0 && entry->e_value_inum == 0) {
@@ -539,8 +539,10 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
        if (error)
                goto cleanup;
        size = le32_to_cpu(entry->e_value_size);
+       error = -ERANGE;
+       if (unlikely(size > EXT4_XATTR_SIZE_MAX))
+               goto cleanup;
        if (buffer) {
-               error = -ERANGE;
                if (size > buffer_size)
                        goto cleanup;
                if (entry->e_value_inum) {
@@ -549,8 +551,12 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
                        if (error)
                                goto cleanup;
                } else {
-                       memcpy(buffer, bh->b_data +
-                              le16_to_cpu(entry->e_value_offs), size);
+                       u16 offset = le16_to_cpu(entry->e_value_offs);
+                       void *p = bh->b_data + offset;
+
+                       if (unlikely(p + size > end))
+                               goto cleanup;
+                       memcpy(buffer, p, size);
                }
        }
        error = size;
@@ -588,8 +594,10 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
        if (error)
                goto cleanup;
        size = le32_to_cpu(entry->e_value_size);
+       error = -ERANGE;
+       if (unlikely(size > EXT4_XATTR_SIZE_MAX))
+               goto cleanup;
        if (buffer) {
-               error = -ERANGE;
                if (size > buffer_size)
                        goto cleanup;
                if (entry->e_value_inum) {
@@ -598,8 +606,12 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
                        if (error)
                                goto cleanup;
                } else {
-                       memcpy(buffer, (void *)IFIRST(header) +
-                              le16_to_cpu(entry->e_value_offs), size);
+                       u16 offset = le16_to_cpu(entry->e_value_offs);
+                       void *p = (void *)IFIRST(header) + offset;
+
+                       if (unlikely(p + size > end))
+                               goto cleanup;
+                       memcpy(buffer, p, size);
                }
        }
        error = size;

diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index f8cc07588ac91ce449017f4d354a90edd0f1f336..262b9314119f6d72a792a3d5a51a4dce04530b21 100644 (file)
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -70,6 +70,17 @@ struct ext4_xattr_entry {
                EXT4_I(inode)->i_extra_isize))
 #define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1))
 
+/*
+ * XATTR_SIZE_MAX is currently 64k, but for the purposes of checking
+ * for file system consistency errors, we use a somewhat bigger value.
+ * This allows XATTR_SIZE_MAX to grow in the future, but by using this
+ * instead of INT_MAX for certain consistency checks, we don't need to
+ * worry about arithmetic overflows.  (Actually XATTR_SIZE_MAX is
+ * defined in include/uapi/linux/limits.h, so changing it is going
+ * not going to be trivial....)
+ */
+#define EXT4_XATTR_SIZE_MAX (1 << 24)
+
 /*
  * The minimum size of EA value when you start storing it in an external inode
  * size of block - size of header - size of 1 entry - 4 null bytes