lkml.org 
[lkml]   [2017]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    To verify that a xattr entry is not pointing to the wrong xattr inode,
    we currently check that the target inode has EXT4_EA_INODE_FL flag set and
    also the entry size matches the target inode size.

    For stronger validation, also incorporate crc32c hash of the value into
    the e_hash field. This is done regardless of whether the entry lives in
    the inode body or external attribute block.

    Signed-off-by: Tahsin Erdogan <tahsin@google.com>
    ---
    v2: naming updates to adapt to other patch changes in the series

    fs/ext4/xattr.c | 104 +++++++++++++++++++++++++++++++++++---------------------
    1 file changed, 65 insertions(+), 39 deletions(-)

    diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
    index 9cb15f2069bd..3f16dc979012 100644
    --- a/fs/ext4/xattr.c
    +++ b/fs/ext4/xattr.c
    @@ -77,8 +77,8 @@ static void ext4_xattr_block_cache_insert(struct mb_cache *,
    static struct buffer_head *
    ext4_xattr_block_cache_find(struct inode *, struct ext4_xattr_header *,
    struct mb_cache_entry **);
    -static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
    - void *value_base);
    +static __le32 ext4_xattr_hash_entry(char *name, size_t name_len, __le32 *value,
    + size_t value_count);
    static void ext4_xattr_rehash(struct ext4_xattr_header *);

    static const struct xattr_handler * const ext4_xattr_handler_map[] = {
    @@ -380,7 +380,9 @@ static int ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
    }

    static int
    -ext4_xattr_inode_verify_hash(struct inode *ea_inode, void *buffer, size_t size)
    +ext4_xattr_inode_verify_hashes(struct inode *ea_inode,
    + struct ext4_xattr_entry *entry, void *buffer,
    + size_t size)
    {
    u32 hash;

    @@ -388,23 +390,35 @@ ext4_xattr_inode_verify_hash(struct inode *ea_inode, void *buffer, size_t size)
    hash = ext4_xattr_inode_hash(EXT4_SB(ea_inode->i_sb), buffer, size);
    if (hash != ext4_xattr_inode_get_hash(ea_inode))
    return -EFSCORRUPTED;
    +
    + if (entry) {
    + __le32 e_hash, tmp_data;
    +
    + /* Verify entry hash. */
    + tmp_data = cpu_to_le32(hash);
    + e_hash = ext4_xattr_hash_entry(entry->e_name, entry->e_name_len,
    + &tmp_data, 1);
    + if (e_hash != entry->e_hash)
    + return -EFSCORRUPTED;
    + }
    return 0;
    }

    #define EXT4_XATTR_INODE_GET_PARENT(inode) ((__u32)(inode)->i_mtime.tv_sec)

    /*
    - * Read the value from the EA inode.
    + * Read xattr value from the EA inode.
    */
    static int
    -ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
    - size_t size)
    +ext4_xattr_inode_get(struct inode *inode, struct ext4_xattr_entry *entry,
    + void *buffer, size_t size)
    {
    struct mb_cache *ea_inode_cache = EA_INODE_CACHE(inode);
    struct inode *ea_inode;
    int err;

    - err = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
    + err = ext4_xattr_inode_iget(inode, le32_to_cpu(entry->e_value_inum),
    + &ea_inode);
    if (err) {
    ea_inode = NULL;
    goto out;
    @@ -422,7 +436,7 @@ ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
    if (err)
    goto out;

    - err = ext4_xattr_inode_verify_hash(ea_inode, buffer, size);
    + err = ext4_xattr_inode_verify_hashes(ea_inode, entry, buffer, size);
    /*
    * Compatibility check for old Lustre ea_inode implementation. Old
    * version does not have hash validation, but it has a backpointer
    @@ -489,9 +503,8 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
    if (size > buffer_size)
    goto cleanup;
    if (entry->e_value_inum) {
    - error = ext4_xattr_inode_get(inode,
    - le32_to_cpu(entry->e_value_inum),
    - buffer, size);
    + error = ext4_xattr_inode_get(inode, entry, buffer,
    + size);
    if (error)
    goto cleanup;
    } else {
    @@ -539,9 +552,8 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
    if (size > buffer_size)
    goto cleanup;
    if (entry->e_value_inum) {
    - error = ext4_xattr_inode_get(inode,
    - le32_to_cpu(entry->e_value_inum),
    - buffer, size);
    + error = ext4_xattr_inode_get(inode, entry, buffer,
    + size);
    if (error)
    goto cleanup;
    } else {
    @@ -1387,8 +1399,8 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
    (EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL) &&
    i_size_read(ea_inode) == value_len &&
    !ext4_xattr_inode_read(ea_inode, ea_data, value_len) &&
    - !ext4_xattr_inode_verify_hash(ea_inode, ea_data,
    - value_len) &&
    + !ext4_xattr_inode_verify_hashes(ea_inode, NULL, ea_data,
    + value_len) &&
    !memcmp(value, ea_data, value_len)) {
    mb_cache_entry_touch(ea_inode_cache, ce);
    mb_cache_entry_put(ea_inode_cache, ce);
    @@ -1652,12 +1664,36 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
    here->e_value_size = cpu_to_le32(i->value_len);
    }

    - if (is_block) {
    - if (i->value)
    - ext4_xattr_hash_entry(here, s->base);
    - ext4_xattr_rehash((struct ext4_xattr_header *)s->base);
    + if (i->value) {
    + __le32 hash = 0;
    +
    + /* Entry hash calculation. */
    + if (in_inode) {
    + __le32 crc32c_hash;
    +
    + /*
    + * Feed crc32c hash instead of the raw value for entry
    + * hash calculation. This is to avoid walking
    + * potentially long value buffer again.
    + */
    + crc32c_hash = cpu_to_le32(
    + ext4_xattr_inode_get_hash(new_ea_inode));
    + hash = ext4_xattr_hash_entry(here->e_name,
    + here->e_name_len,
    + &crc32c_hash, 1);
    + } else if (is_block) {
    + __le32 *value = s->base + min_offs - new_size;
    +
    + hash = ext4_xattr_hash_entry(here->e_name,
    + here->e_name_len, value,
    + new_size >> 2);
    + }
    + here->e_hash = hash;
    }

    + if (is_block)
    + ext4_xattr_rehash((struct ext4_xattr_header *)s->base);
    +
    ret = 0;
    out:
    iput(old_ea_inode);
    @@ -2439,9 +2475,7 @@ static int ext4_xattr_move_to_block(handle_t *handle, struct inode *inode,

    /* Save the entry name and the entry value */
    if (entry->e_value_inum) {
    - error = ext4_xattr_inode_get(inode,
    - le32_to_cpu(entry->e_value_inum),
    - buffer, value_size);
    + error = ext4_xattr_inode_get(inode, entry, buffer, value_size);
    if (error)
    goto out;
    } else {
    @@ -2931,30 +2965,22 @@ ext4_xattr_block_cache_find(struct inode *inode,
    *
    * Compute the hash of an extended attribute.
    */
    -static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
    - void *value_base)
    +static __le32 ext4_xattr_hash_entry(char *name, size_t name_len, __le32 *value,
    + size_t value_count)
    {
    __u32 hash = 0;
    - char *name = entry->e_name;
    - int n;

    - for (n = 0; n < entry->e_name_len; n++) {
    + while (name_len--) {
    hash = (hash << NAME_HASH_SHIFT) ^
    (hash >> (8*sizeof(hash) - NAME_HASH_SHIFT)) ^
    *name++;
    }
    -
    - if (!entry->e_value_inum && entry->e_value_size) {
    - __le32 *value = (__le32 *)((char *)value_base +
    - le16_to_cpu(entry->e_value_offs));
    - for (n = (le32_to_cpu(entry->e_value_size) +
    - EXT4_XATTR_ROUND) >> EXT4_XATTR_PAD_BITS; n; n--) {
    - hash = (hash << VALUE_HASH_SHIFT) ^
    - (hash >> (8*sizeof(hash) - VALUE_HASH_SHIFT)) ^
    - le32_to_cpu(*value++);
    - }
    + while (value_count--) {
    + hash = (hash << VALUE_HASH_SHIFT) ^
    + (hash >> (8*sizeof(hash) - VALUE_HASH_SHIFT)) ^
    + le32_to_cpu(*value++);
    }
    - entry->e_hash = cpu_to_le32(hash);
    + return cpu_to_le32(hash);
    }

    #undef NAME_HASH_SHIFT
    --
    2.13.1.611.g7e3b11ae1-goog
    \
     
     \ /
      Last update: 2017-06-22 20:00    [W:4.273 / U:0.064 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site