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 <tah...@google.com> --- fs/ext4/xattr.c | 106 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 39 deletions(-) diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 6c6dce2f874e..7e9134fb4bb8 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[] = { @@ -389,18 +389,20 @@ static int ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino, } /* - * 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; + __le32 calc_e_hash, tmp_data; u32 hash; 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; @@ -415,11 +417,25 @@ ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer, } err = ext4_xattr_inode_read(ea_inode, buffer, size); - if (!err) { - hash = ext4_xattr_inode_get_hash(ea_inode); - mb_cache_entry_create(ea_inode_cache, GFP_NOFS, hash, - ea_inode->i_ino, true /* reusable */); + if (err) + goto out; + + hash = ext4_xattr_inode_get_hash(ea_inode); + + /* Verify e_hash. */ + tmp_data = cpu_to_le32(hash); + calc_e_hash = ext4_xattr_hash_entry(entry->e_name, entry->e_name_len, + &tmp_data, 1); + if (calc_e_hash != entry->e_hash) { + ext4_warning_inode(inode, + "Entry calc_e_hash=%#x does not match e_hash=%#x", + le32_to_cpu(calc_e_hash), le32_to_cpu(entry->e_hash)); + err = -EFSCORRUPTED; + goto out; } + + mb_cache_entry_create(ea_inode_cache, GFP_NOFS, hash, ea_inode->i_ino, + true /* reusable */); out: iput(ea_inode); return err; @@ -465,9 +481,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 { @@ -515,9 +530,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 { @@ -1634,12 +1648,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 (s->not_found || 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); @@ -2421,9 +2459,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 { @@ -2913,30 +2949,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.508.gb3defc5cc-goog ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Jfs-discussion mailing list Jfs-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jfs-discussion