ext4: add extra checks to ext4_xattr_block_get()

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

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 6304e81bfe6a800c93ba053ce939fcd84a672a77..499cb4b1fbd22b98a8b8d923910ed0cce72d6994 100644 (file)
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -197,7 +197,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) {
@@ -540,8 +540,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) {
@@ -550,8 +552,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;
@@ -589,8 +595,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) {
@@ -599,8 +607,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 dd54c4f995c8d0439c7647963cd8394b3f5e94e0..f39cad2abe2a8855211c07b22938499ba1d3a40e 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