add-extra-checks-to-ext4_xattr_get-functions

   1 ext4: add extra checks to ext4_xattr_block_get()
   2
   3 Add explicit checks in ext4_xattr_block_get() just in case the
   4 e_value_offs and e_value_size fields in the the xattr block are
   5 corrupted in memory after the buffer_verified bit is set on the xattr
   6 block.
   7
   8 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
   9 Cc: stable@kernel.org
  10 ---
  11  fs/ext4/xattr.c | 26 +++++++++++++++++++-------
  12  fs/ext4/xattr.h | 11 +++++++++++
  13  2 files changed, 30 insertions(+), 7 deletions(-)
  14
  15 diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
  16 index 9b9ba7c85f7a..c5d0c3fad560 100644
  17 --- a/fs/ext4/xattr.c
  18 +++ b/fs/ext4/xattr.c
  19 @@ -197,7 +197,7 @@ ext4_xattr_check_entries(struct ext4_xattr_entry *entry, void *end,
  20         while (!IS_LAST_ENTRY(entry)) {
  21                 u32 size = le32_to_cpu(entry->e_value_size);
  22
  23 -               if (size > INT_MAX)
  24 +               if (size > EXT4_XATTR_SIZE_MAX)
  25                         return -EFSCORRUPTED;
  26
  27                 if (entry->e_value_size != 0 &&
  28 @@ -541,8 +541,10 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
  29         if (error)
  30                 goto cleanup;
  31         size = le32_to_cpu(entry->e_value_size);
  32 +       error = -ERANGE;
  33 +       if (unlikely(size > EXT4_XATTR_SIZE_MAX))
  34 +               goto cleanup;
  35         if (buffer) {
  36 -               error = -ERANGE;
  37                 if (size > buffer_size)
  38                         goto cleanup;
  39                 if (entry->e_value_inum) {
  40 @@ -551,8 +553,12 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
  41                         if (error)
  42                                 goto cleanup;
  43                 } else {
  44 -                       memcpy(buffer, bh->b_data +
  45 -                              le16_to_cpu(entry->e_value_offs), size);
  46 +                       u16 offset = le16_to_cpu(entry->e_value_offs);
  47 +                       void *p = bh->b_data + offset;
  48 +
  49 +                       if (unlikely(p + size > end))
  50 +                               goto cleanup;
  51 +                       memcpy(buffer, p, size);
  52                 }
  53         }
  54         error = size;
  55 @@ -590,8 +596,10 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
  56         if (error)
  57                 goto cleanup;
  58         size = le32_to_cpu(entry->e_value_size);
  59 +       error = -ERANGE;
  60 +       if (unlikely(size > EXT4_XATTR_SIZE_MAX))
  61 +               goto cleanup;
  62         if (buffer) {
  63 -               error = -ERANGE;
  64                 if (size > buffer_size)
  65                         goto cleanup;
  66                 if (entry->e_value_inum) {
  67 @@ -600,8 +608,12 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
  68                         if (error)
  69                                 goto cleanup;
  70                 } else {
  71 -                       memcpy(buffer, (void *)IFIRST(header) +
  72 -                              le16_to_cpu(entry->e_value_offs), size);
  73 +                       u16 offset = le16_to_cpu(entry->e_value_offs);
  74 +                       void *p = (void *)IFIRST(header) + offset;
  75 +
  76 +                       if (unlikely(p + size > end))
  77 +                               goto cleanup;
  78 +                       memcpy(buffer, p, size);
  79                 }
  80         }
  81         error = size;
  82 diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
  83 index dd54c4f995c8..f39cad2abe2a 100644
  84 --- a/fs/ext4/xattr.h
  85 +++ b/fs/ext4/xattr.h
  86 @@ -70,6 +70,17 @@ struct ext4_xattr_entry {
  87                 EXT4_I(inode)->i_extra_isize))
  88  #define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1))
  89
  90 +/*
  91 + * XATTR_SIZE_MAX is currently 64k, but for the purposes of checking
  92 + * for file system consistency errors, we use a somewhat bigger value.
  93 + * This allows XATTR_SIZE_MAX to grow in the future, but by using this
  94 + * instead of INT_MAX for certain consistency checks, we don't need to
  95 + * worry about arithmetic overflows.  (Actually XATTR_SIZE_MAX is
  96 + * defined in include/uapi/linux/limits.h, so changing it is going
  97 + * not going to be trivial....)
  98 + */
  99 +#define EXT4_XATTR_SIZE_MAX (1 << 24)
 100 +
 101  /*
 102   * The minimum size of EA value when you start storing it in an external inode
 103   * size of block - size of header - size of 1 entry - 4 null bytes