]> git.ipfire.org Git - thirdparty/e2fsprogs.git/commitdiff
e2fsck: ea_inode hash validation
authorTahsin Erdogan <tahsin@google.com>
Wed, 5 Jul 2017 03:53:59 +0000 (23:53 -0400)
committerTheodore Ts'o <tytso@mit.edu>
Wed, 5 Jul 2017 03:53:59 +0000 (23:53 -0400)
In original implementation of ea_inode feature, each xattr inode had
a single parent. Child inode tracked the parent by storing its inode
number in i_mtime field. Also, child's i_generation matched parent's
i_generation.

With deduplication support, xattr inodes can now be shared so a single
backpointer is not sufficient to achieve strong binding. This is now
replaced by hash validation.

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
e2fsck/pass1.c
e2fsck/problem.c
e2fsck/problem.h
lib/ext2fs/ext2fs.h
lib/ext2fs/ext_attr.c

index 8780af0c0e2db9a80996fd91f547fa9676ae29a9..40ee7fdae60a15781ad537cb9d27d6174657a23e 100644 (file)
@@ -360,27 +360,54 @@ static problem_t check_large_ea_inode(e2fsck_t ctx,
                                      struct problem_context *pctx)
 {
        struct ext2_inode inode;
+       __u32 hash;
+       errcode_t retval;
 
        /* Check if inode is within valid range */
        if ((entry->e_value_inum < EXT2_FIRST_INODE(ctx->fs->super)) ||
-           (entry->e_value_inum > ctx->fs->super->s_inodes_count))
+           (entry->e_value_inum > ctx->fs->super->s_inodes_count)) {
+               pctx->num = entry->e_value_inum;
                return PR_1_ATTR_VALUE_EA_INODE;
+       }
 
        e2fsck_read_inode(ctx, entry->e_value_inum, &inode, "pass1");
+
+       retval = ext2fs_ext_attr_hash_entry2(ctx->fs, entry, NULL, &hash);
+       if (retval) {
+               com_err("check_large_ea_inode", retval,
+                       _("while hashing entry with e_value_inum = %u"),
+                       entry->e_value_inum);
+               fatal_error(ctx, 0);
+       }
+
+       if (hash != entry->e_hash) {
+               /* This might be an old Lustre-style ea_inode reference. */
+               if (inode.i_mtime != pctx->ino ||
+                   inode.i_generation != pctx->inode->i_generation) {
+
+                       /* If target inode is also missing EA_INODE flag,
+                        * this is likely to be a bad reference.
+                        */
+                       if (!(inode.i_flags & EXT4_EA_INODE_FL)) {
+                               pctx->num = entry->e_value_inum;
+                               return PR_1_ATTR_VALUE_EA_INODE;
+                       } else {
+                               pctx->num = entry->e_hash;
+                               return PR_1_ATTR_HASH;
+                       }
+               }
+       }
+
        if (!(inode.i_flags & EXT4_EA_INODE_FL)) {
-               /* If EXT4_EA_INODE_FL flag is not present but back-pointer
-                * matches then we should set this flag */
-               if (inode.i_mtime == pctx->ino &&
-                   inode.i_generation == pctx->inode->i_generation &&
-                   fix_problem(ctx, PR_1_ATTR_SET_EA_INODE_FL, pctx)) {
+               pctx->num = entry->e_value_inum;
+               if (fix_problem(ctx, PR_1_ATTR_SET_EA_INODE_FL, pctx)) {
                        inode.i_flags |= EXT4_EA_INODE_FL;
-                       ext2fs_write_inode(ctx->fs, entry->e_value_inum,&inode);
-               } else
+                       ext2fs_write_inode(ctx->fs, entry->e_value_inum,
+                                          &inode);
+               } else {
                        return PR_1_ATTR_NO_EA_INODE_FL;
-       } else if (inode.i_mtime != pctx->ino ||
-                  inode.i_generation != pctx->inode->i_generation)
-               return PR_1_ATTR_INVAL_EA_INODE;
-
+               }
+       }
        return 0;
 }
 
@@ -458,6 +485,16 @@ static void check_ea_in_inode(e2fsck_t ctx, struct problem_context *pctx)
                                problem = PR_1_INODE_EA_ALLOC_COLLISION;
                                goto fix;
                        }
+
+                       hash = ext2fs_ext_attr_hash_entry(entry,
+                                                         start + entry->e_value_offs);
+
+                       /* e_hash may be 0 in older inode's ea */
+                       if (entry->e_hash != 0 && entry->e_hash != hash) {
+                               pctx->num = entry->e_hash;
+                               problem = PR_1_ATTR_HASH;
+                               goto fix;
+                       }
                } else {
                        problem = check_large_ea_inode(ctx, entry, pctx);
                        if (problem != 0)
@@ -466,16 +503,6 @@ static void check_ea_in_inode(e2fsck_t ctx, struct problem_context *pctx)
                        mark_inode_ea_map(ctx, pctx, entry->e_value_inum);
                }
 
-               hash = ext2fs_ext_attr_hash_entry(entry,
-                                                 start + entry->e_value_offs);
-
-               /* e_hash may be 0 in older inode's ea */
-               if (entry->e_hash != 0 && entry->e_hash != hash) {
-                       pctx->num = entry->e_hash;
-                       problem = PR_1_ATTR_HASH;
-                       goto fix;
-               }
-
                /* If EA value is stored in external inode then it does not
                 * consume space here */
                if (entry->e_value_inum == 0)
@@ -2439,6 +2466,16 @@ static int check_ext_attr(e2fsck_t ctx, struct problem_context *pctx,
                                                pctx))
                                        goto clear_extattr;
                        }
+
+                       hash = ext2fs_ext_attr_hash_entry(entry, block_buf +
+                                                         entry->e_value_offs);
+
+                       if (entry->e_hash != hash) {
+                               pctx->num = entry->e_hash;
+                               if (fix_problem(ctx, PR_1_ATTR_HASH, pctx))
+                                       goto clear_extattr;
+                               entry->e_hash = hash;
+                       }
                } else {
                        problem_t problem;
 
@@ -2450,16 +2487,6 @@ static int check_ext_attr(e2fsck_t ctx, struct problem_context *pctx,
                                goto clear_extattr;
                }
 
-               hash = ext2fs_ext_attr_hash_entry(entry, block_buf +
-                                                        entry->e_value_offs);
-
-               if (entry->e_hash != hash) {
-                       pctx->num = entry->e_hash;
-                       if (fix_problem(ctx, PR_1_ATTR_HASH, pctx))
-                               goto clear_extattr;
-                       entry->e_hash = hash;
-               }
-
                entry = EXT2_EXT_ATTR_NEXT(entry);
        }
        if (region_allocate(region, (char *)entry - (char *)header, 4)) {
index dcdf9abe1abded843b56234b2ddc3c54441a3ae0..deffa4d66a892552ea69aa7873965f552ca3a3e0 100644 (file)
@@ -1150,11 +1150,6 @@ static struct e2fsck_problem problem_table[] = {
          N_("@i %i has @I @a value @i %N.\n"),
          PROMPT_CLEAR, PR_PREEN_OK },
 
-       /* Invalid backpointer from extended attribute inode to parent inode */
-       { PR_1_ATTR_INVAL_EA_INODE,
-         N_("@n backpointer from @a @i %N to parent @i %i.\n"),
-         PROMPT_CLEAR, PR_PREEN_OK },
-
        /* Inode has invalid extended attribute. EA inode missing
         * EA_INODE flag. */
        { PR_1_ATTR_NO_EA_INODE_FL,
index dad608c94fd06d812f7f5535443a6b178dfee423..60cc764fd2dfec9e6ff3aa8e7fa8d4521840d646 100644 (file)
@@ -678,9 +678,6 @@ struct problem_context {
 /* Inode has illegal EA value inode */
 #define PR_1_ATTR_VALUE_EA_INODE               0x010083
 
-/* Invalid backpointer from EA inode to parent inode */
-#define PR_1_ATTR_INVAL_EA_INODE               0x010084
-
 /* Parent inode has invalid EA entry. EA inode does not have
  * EXT4_EA_INODE_FL flag. Delete EA entry? */
 #define PR_1_ATTR_NO_EA_INODE_FL               0x010085
index f4131a640f9c615cdfc11d83abbe49a927e2eab1..f72cbe17a8cd05e40683c564e16643bab5db15a7 100644 (file)
@@ -1198,6 +1198,9 @@ extern errcode_t ext2fs_expand_dir(ext2_filsys fs, ext2_ino_t dir);
 /* ext_attr.c */
 extern __u32 ext2fs_ext_attr_hash_entry(struct ext2_ext_attr_entry *entry,
                                        void *data);
+extern errcode_t ext2fs_ext_attr_hash_entry2(ext2_filsys fs,
+                                            struct ext2_ext_attr_entry *entry,
+                                            void *data, __u32 *hash);
 extern errcode_t ext2fs_read_ext_attr(ext2_filsys fs, blk_t block, void *buf);
 extern errcode_t ext2fs_read_ext_attr2(ext2_filsys fs, blk64_t block,
                                       void *buf);
index 14d05c7e57fbe93f68706c45d8a50d0e5285c6fd..d2ce51bcdcd8d8396118ce41c11de8bb3365f8dc 100644 (file)
 
 #include "ext2fs.h"
 
+static errcode_t read_ea_inode_hash(ext2_filsys fs, ext2_ino_t ino, __u32 *hash)
+{
+       struct ext2_inode inode;
+       errcode_t retval;
+
+       retval = ext2fs_read_inode(fs, ino, &inode);
+       if (retval)
+               return retval;
+       *hash = inode.i_atime;
+       return 0;
+}
+
 #define NAME_HASH_SHIFT 5
 #define VALUE_HASH_SHIFT 16
 
@@ -59,6 +71,35 @@ __u32 ext2fs_ext_attr_hash_entry(struct ext2_ext_attr_entry *entry, void *data)
        return hash;
 }
 
+/*
+ * ext2fs_ext_attr_hash_entry2()
+ *
+ * Compute the hash of an extended attribute.
+ * This version of the function supports hashing entries that reference
+ * external inodes (ea_inode feature).
+ */
+errcode_t ext2fs_ext_attr_hash_entry2(ext2_filsys fs,
+                                     struct ext2_ext_attr_entry *entry,
+                                     void *data, __u32 *hash)
+{
+       *hash = ext2fs_ext_attr_hash_entry(entry, data);
+
+       if (entry->e_value_inum) {
+               __u32 ea_inode_hash;
+               errcode_t retval;
+
+               retval = read_ea_inode_hash(fs, entry->e_value_inum,
+                                           &ea_inode_hash);
+               if (retval)
+                       return retval;
+
+               *hash = (*hash << VALUE_HASH_SHIFT) ^
+                       (*hash >> (8*sizeof(*hash) - VALUE_HASH_SHIFT)) ^
+                       ea_inode_hash;
+       }
+       return 0;
+}
+
 static errcode_t check_ext_attr_header(struct ext2_ext_attr_header *header)
 {
        if ((header->h_magic != EXT2_EXT_ATTR_MAGIC_v1 &&
@@ -785,6 +826,7 @@ out:
 }
 
 static errcode_t read_xattrs_from_buffer(struct ext2_xattr_handle *handle,
+                                        struct ext2_inode_large *inode,
                                         struct ext2_ext_attr_entry *entries,
                                         unsigned int storage_size,
                                         char *value_start,
@@ -914,9 +956,25 @@ static errcode_t read_xattrs_from_buffer(struct ext2_xattr_handle *handle,
                        void *data = (entry->e_value_inum != 0) ?
                                        0 : value_start + entry->e_value_offs;
 
-                       hash = ext2fs_ext_attr_hash_entry(entry, data);
-                       if (entry->e_hash != hash)
-                               return EXT2_ET_BAD_EA_HASH;
+                       err = ext2fs_ext_attr_hash_entry2(handle->fs, entry,
+                                                         data, &hash);
+                       if (err)
+                               return err;
+                       if (entry->e_hash != hash) {
+                               struct ext2_inode child;
+
+                               /* Check whether this is an old Lustre-style
+                                * ea_inode reference.
+                                */
+                               err = ext2fs_read_inode(handle->fs,
+                                                       entry->e_value_inum,
+                                                       &child);
+                               if (err)
+                                       return err;
+                               if (child.i_mtime != handle->ino ||
+                                   child.i_generation != inode->i_generation)
+                                       return EXT2_ET_BAD_EA_HASH;
+                       }
                }
 
                x++;
@@ -989,7 +1047,7 @@ errcode_t ext2fs_xattrs_read(struct ext2_xattr_handle *handle)
                start = ((char *) inode) + EXT2_GOOD_OLD_INODE_SIZE +
                        inode->i_extra_isize + sizeof(__u32);
 
-               err = read_xattrs_from_buffer(handle,
+               err = read_xattrs_from_buffer(handle, inode,
                        (struct ext2_ext_attr_entry *) start, storage_size,
                                              start, &handle->count);
                if (err)
@@ -1026,7 +1084,7 @@ read_ea_block:
                storage_size = handle->fs->blocksize -
                        sizeof(struct ext2_ext_attr_header);
                start = block_buf + sizeof(struct ext2_ext_attr_header);
-               err = read_xattrs_from_buffer(handle,
+               err = read_xattrs_from_buffer(handle, inode,
                        (struct ext2_ext_attr_entry *) start, storage_size,
                                              block_buf, &handle->count);
                if (err)