From 1fa48473e79dca213e7debad20b4d3b1e553464a Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 22 Jun 2017 11:54:08 -0400 Subject: [PATCH] add patch strong-binding-of-xattr-inode-references --- series | 1 + strong-binding-of-xattr-inode-references | 225 +++++++++++++++++++++++++++++++ timestamps | 5 +- 3 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 strong-binding-of-xattr-inode-references diff --git a/series b/series index ab5e2583..9facc4a1 100644 --- a/series +++ b/series @@ -31,6 +31,7 @@ xattr-inode-deduplication add-get_inode_usage-callback-to-transfer-multi-inode-charges reserve-space-for-xattr-entries_names eliminate-xattr-entry-e_hash-recalcuation-for-removes +strong-binding-of-xattr-inode-references #################################################### # unstable patches diff --git a/strong-binding-of-xattr-inode-references b/strong-binding-of-xattr-inode-references new file mode 100644 index 00000000..e98eccad --- /dev/null +++ b/strong-binding-of-xattr-inode-references @@ -0,0 +1,225 @@ +ext4: strong binding of xattr inode references + +From: Tahsin Erdogan + +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 +Signed-off-by: Theodore Ts'o +--- +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 + + diff --git a/timestamps b/timestamps index 175c87fb..f52c9944 100755 --- a/timestamps +++ b/timestamps @@ -63,7 +63,8 @@ touch -d @1498146129 cleanup-transaction-restarts-during-inode-deletion touch -d @1498146295 xattr-inode-deduplication touch -d @1498146408 add-get_inode_usage-callback-to-transfer-multi-inode-charges touch -d @1498146533 reserve-space-for-xattr-entries_names -touch -d @1498146710 series touch -d @1498146723 eliminate-xattr-entry-e_hash-recalcuation-for-removes touch -d @1498146730 status -touch -d @1498146731 timestamps +touch -d @1498146795 strong-binding-of-xattr-inode-references +touch -d @1498146817 series +touch -d @1498146833 timestamps -- 2.11.4.GIT