e2fsck: add more checks for ea inode consistency

Currently checking of EA inodes was rather weak. Add several more
consistency checks.

1) Check that EA inode is a regular file.
2) Check that EA_INODE feature is set if the filesystem has EA inodes.
3) Make sure that no EA inode is referenced from directory hierarchy.

Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/r/20240506174132.12883-1-jack@suse.cz
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
index 55738fd..ae1273d 100644 (file)
--- a/e2fsck/e2fsck.h
+++ b/e2fsck/e2fsck.h
@@ -533,6 +533,12 @@ extern struct dx_dir_info *e2fsck_dx_dir_info_iter(e2fsck_t ctx,
 typedef __u64 ea_key_t;
 typedef __u64 ea_value_t;
 
+/*
+ * Special refcount value we use for inodes which have EA_INODE flag set but we
+ * do not yet know about any references.
+ */
+#define EA_INODE_NO_REFS (~(ea_value_t)0)
+
 extern errcode_t ea_refcount_create(size_t size, ext2_refcount_t *ret);
 extern void ea_refcount_free(ext2_refcount_t refcount);
 extern errcode_t ea_refcount_fetch(ext2_refcount_t refcount, ea_key_t ea_key,

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 8b6238e..eb73922 100644 (file)
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -387,34 +387,71 @@ static problem_t check_large_ea_inode(e2fsck_t ctx,
        return 0;
 }
 
+static int alloc_ea_inode_refs(e2fsck_t ctx, struct problem_context *pctx)
+{
+       pctx->errcode = ea_refcount_create(0, &ctx->ea_inode_refs);
+       if (pctx->errcode) {
+               pctx->num = 4;
+               fix_problem(ctx, PR_1_ALLOCATE_REFCOUNT, pctx);
+               ctx->flags |= E2F_FLAG_ABORT;
+               return 0;
+       }
+       return 1;
+}
+
 static void inc_ea_inode_refs(e2fsck_t ctx, struct problem_context *pctx,
                              struct ext2_ext_attr_entry *first, void *end)
 {
        struct ext2_ext_attr_entry *entry = first;
        struct ext2_ext_attr_entry *np = EXT2_EXT_ATTR_NEXT(entry);
+       ea_value_t refs;
 
        while ((void *) entry < end && (void *) np < end &&
               !EXT2_EXT_IS_LAST_ENTRY(entry)) {
                if (!entry->e_value_inum)
                        goto next;
-               if (!ctx->ea_inode_refs) {
-                       pctx->errcode = ea_refcount_create(0,
-                                                          &ctx->ea_inode_refs);
-                       if (pctx->errcode) {
-                               pctx->num = 4;
-                               fix_problem(ctx, PR_1_ALLOCATE_REFCOUNT, pctx);
-                               ctx->flags |= E2F_FLAG_ABORT;
-                               return;
-                       }
-               }
-               ea_refcount_increment(ctx->ea_inode_refs, entry->e_value_inum,
-                                     0);
+               if (!ctx->ea_inode_refs && !alloc_ea_inode_refs(ctx, pctx))
+                       return;
+               ea_refcount_fetch(ctx->ea_inode_refs, entry->e_value_inum,
+                                 &refs);
+               if (refs == EA_INODE_NO_REFS)
+                       refs = 1;
+               else
+                       refs += 1;
+               ea_refcount_store(ctx->ea_inode_refs, entry->e_value_inum, refs);
        next:
                entry = np;
                np = EXT2_EXT_ATTR_NEXT(entry);
        }
 }
 
+/*
+ * Make sure inode is tracked as EA inode. We use special EA_INODE_NO_REFS
+ * value if we didn't find any xattrs referencing this inode yet.
+ */
+static int track_ea_inode(e2fsck_t ctx, struct problem_context *pctx,
+                         ext2_ino_t ino)
+{
+       ea_value_t refs;
+
+       if (!ctx->ea_inode_refs && !alloc_ea_inode_refs(ctx, pctx))
+               return 0;
+
+       ea_refcount_fetch(ctx->ea_inode_refs, ino, &refs);
+       if (refs > 0)
+               return 1;
+
+       pctx->errcode = ea_refcount_store(ctx->ea_inode_refs, ino,
+                                         EA_INODE_NO_REFS);
+       if (pctx->errcode) {
+               pctx->num = 5;
+               fix_problem(ctx, PR_1_ALLOCATE_REFCOUNT, pctx);
+               ctx->flags |= E2F_FLAG_ABORT;
+               return 0;
+       }
+       return 1;
+}
+
 static void check_ea_in_inode(e2fsck_t ctx, struct problem_context *pctx,
                              struct ea_quota *ea_ibody_quota)
 {
@@ -510,6 +547,12 @@ static void check_ea_in_inode(e2fsck_t ctx, struct problem_context *pctx,
                } else {
                        blk64_t quota_blocks;
 
+                       if (!ext2fs_has_feature_ea_inode(sb) &&
+                           fix_problem(ctx, PR_1_EA_INODE_FEATURE, pctx)) {
+                               ext2fs_set_feature_ea_inode(sb);
+                               ext2fs_mark_super_dirty(ctx->fs);
+                       }
+
                        problem = check_large_ea_inode(ctx, entry, pctx,
                                                       &quota_blocks);
                        if (problem != 0)
@@ -1502,6 +1545,17 @@ void e2fsck_pass1(e2fsck_t ctx)
                        e2fsck_write_inode(ctx, ino, inode, "pass1");
                }
 
+               if (inode->i_flags & EXT4_EA_INODE_FL) {
+                       if (!LINUX_S_ISREG(inode->i_mode) &&
+                           fix_problem(ctx, PR_1_EA_INODE_NONREG, &pctx)) {
+                               inode->i_flags &= ~EXT4_EA_INODE_FL;
+                               e2fsck_write_inode(ctx, ino, inode, "pass1");
+                       }
+                       if (inode->i_flags & EXT4_EA_INODE_FL)
+                               if (!track_ea_inode(ctx, &pctx, ino))
+                                       continue;
+               }
+
                /* Conflicting inlinedata/extents inode flags? */
                if ((inode->i_flags & EXT4_INLINE_DATA_FL) &&
                    (inode->i_flags & EXT4_EXTENTS_FL)) {
@@ -2622,6 +2676,12 @@ static int check_ext_attr(e2fsck_t ctx, struct problem_context *pctx,
                        problem_t problem;
                        blk64_t entry_quota_blocks;
 
+                       if (!ext2fs_has_feature_ea_inode(fs->super) &&
+                           fix_problem(ctx, PR_1_EA_INODE_FEATURE, pctx)) {
+                               ext2fs_set_feature_ea_inode(fs->super);
+                               ext2fs_mark_super_dirty(fs);
+                       }
+
                        problem = check_large_ea_inode(ctx, entry, pctx,
                                                       &entry_quota_blocks);
                        if (problem && fix_problem(ctx, problem, pctx))

diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
index 08ab40f..036c002 100644 (file)
--- a/e2fsck/pass2.c
+++ b/e2fsck/pass2.c
@@ -1501,6 +1501,21 @@ skip_checksum:
                        problem = PR_2_NULL_NAME;
                }
 
+               /*
+                * Check if inode was tracked as EA inode and has actual
+                * references from xattrs. In that case dir entry is likely
+                * bogus and we want to clear it. The case of EA inode without
+                * references from xattrs will be handled in pass 4.
+                */
+               if (!problem && ctx->ea_inode_refs) {
+                       ea_value_t refs;
+
+                       ea_refcount_fetch(ctx->ea_inode_refs, dirent->inode,
+                                         &refs);
+                       if (refs && refs != EA_INODE_NO_REFS)
+                               problem = PR_2_EA_INODE_DIR_LINK;
+               }
+
                if (problem) {
                        if (fix_problem(ctx, problem, &cd->pctx)) {
                                dirent->inode = 0;

diff --git a/e2fsck/pass4.c b/e2fsck/pass4.c
index d2dda02..cf0cf7c 100644 (file)
--- a/e2fsck/pass4.c
+++ b/e2fsck/pass4.c
@@ -96,9 +96,10 @@ static int disconnect_inode(e2fsck_t ctx, ext2_ino_t i, ext2_ino_t *last_ino,
  * an xattr inode at all. Return immediately if EA_INODE flag is not set.
  */
 static void check_ea_inode(e2fsck_t ctx, ext2_ino_t i, ext2_ino_t *last_ino,
-                          struct ext2_inode_large *inode, __u16 *link_counted)
+                          struct ext2_inode_large *inode, __u16 *link_counted,
+                          ea_value_t actual_refs)
 {
-       __u64 actual_refs = 0;
+       struct problem_context pctx;
        __u64 ref_count;
 
        if (*last_ino != i) {
@@ -107,13 +108,26 @@ static void check_ea_inode(e2fsck_t ctx, ext2_ino_t i, ext2_ino_t *last_ino,
                                       "pass4: check_ea_inode");
                *last_ino = i;
        }
-       if (!(inode->i_flags & EXT4_EA_INODE_FL))
-               return;
 
-       if (ctx->ea_inode_refs)
-               ea_refcount_fetch(ctx->ea_inode_refs, i, &actual_refs);
-       if (!actual_refs)
+       clear_problem_context(&pctx);
+       pctx.ino = i;
+       pctx.inode = EXT2_INODE(inode);
+
+       /* No references to the inode from xattrs? */
+       if (actual_refs == EA_INODE_NO_REFS) {
+               /*
+                * No references from directory hierarchy either? Inode will
+                * will get attached to lost+found so clear EA_INODE_FL.
+                * Otherwise this is likely a spuriously set flag so clear it.
+                */
+               if (*link_counted == 0 ||
+                   fix_problem(ctx, PR_4_EA_INODE_SPURIOUS_FLAG, &pctx)) {
+                       /* Clear EA_INODE_FL (likely a normal file) */
+                       inode->i_flags &= ~EXT4_EA_INODE_FL;
+                       e2fsck_write_inode(ctx, i, EXT2_INODE(inode), "pass4");
+               }
                return;
+       }
 
        /*
         * There are some attribute references, link_counted is now considered
@@ -127,10 +141,6 @@ static void check_ea_inode(e2fsck_t ctx, ext2_ino_t i, ext2_ino_t *last_ino,
         * However, their i_ctime and i_atime should be the same.
         */
        if (ref_count != actual_refs && inode->i_ctime != inode->i_atime) {
-               struct problem_context pctx;
-
-               clear_problem_context(&pctx);
-               pctx.ino = i;
                pctx.num = ref_count;
                pctx.num2 = actual_refs;
                if (fix_problem(ctx, PR_4_EA_INODE_REF_COUNT, &pctx)) {
@@ -188,6 +198,7 @@ void e2fsck_pass4(e2fsck_t ctx)
        /* Protect loop from wrap-around if s_inodes_count maxed */
        for (i = 1; i <= fs->super->s_inodes_count && i > 0; i++) {
                ext2_ino_t last_ino = 0;
+               ea_value_t ea_refs;
                int isdir;
 
                if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
@@ -211,13 +222,19 @@ void e2fsck_pass4(e2fsck_t ctx)
                ext2fs_icount_fetch(ctx->inode_link_info, i, &link_count);
                ext2fs_icount_fetch(ctx->inode_count, i, &link_counted);
 
-               if (link_counted == 0) {
-                       /*
-                        * link_counted is expected to be 0 for an ea_inode.
-                        * check_ea_inode() will update link_counted if
-                        * necessary.
-                        */
-                       check_ea_inode(ctx, i, &last_ino, inode, &link_counted);
+               if (ctx->ea_inode_refs) {
+                       ea_refcount_fetch(ctx->ea_inode_refs, i, &ea_refs);
+                       if (ea_refs) {
+                               /*
+                                * Final consolidation of EA inodes. We either
+                                * decide the inode is fine and set link_counted
+                                * to one, or we decide this is actually a
+                                * normal file and clear EA_INODE flag, or
+                                * decide the inode should just be deleted.
+                                */
+                               check_ea_inode(ctx, i, &last_ino, inode,
+                                              &link_counted, ea_refs);
+                       }
                }
 
                if (link_counted == 0) {

diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index 207ebbb..e433281 100644 (file)
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -1309,6 +1309,16 @@ static struct e2fsck_problem problem_table[] = {
          N_("Orphan file @i %i is not in use, but contains data.  "),
          PROMPT_CLEAR, PR_PREEN_OK },
 
+       /* EA_INODE flag set on a non-regular file */
+       { PR_1_EA_INODE_NONREG,
+         N_("@i %i has the ea_inode flag set but is not a regular file.  "),
+         PROMPT_CLEAR_FLAG, 0, 0, 0, 0 },
+
+       /* EA_INODE present but the file system is missing the ea_inode feature */
+       { PR_1_EA_INODE_FEATURE,
+         N_("@i %i references EA inode but @S is missing EA_INODE feature\n"),
+         PROMPT_FIX, PR_PREEN_OK, 0, 0, 0 },
+
        /* Pass 1b errors */
 
        /* Pass 1B: Rescan for duplicate/bad blocks */
@@ -1860,6 +1870,10 @@ static struct e2fsck_problem problem_table[] = {
           N_("Duplicate filename @E found.  "),
           PROMPT_CLEAR, 0, 0, 0, 0 },
 
+       /* Directory filename is null */
+       { PR_2_EA_INODE_DIR_LINK,
+         N_("@E references EA @i %Di.\n"),
+         PROMPT_CLEAR, 0, 0, 0, 0 },
 
        /* Pass 3 errors */
 
@@ -2102,6 +2116,10 @@ static struct e2fsck_problem problem_table[] = {
          N_("@d @i %i ref count set to overflow but could be exact value %N.  "),
          PROMPT_FIX, PR_PREEN_OK, 0, 0, 0 },
 
+       { PR_4_EA_INODE_SPURIOUS_FLAG,
+         N_("Regular @f @i %i has EA_INODE flag set. "),
+         PROMPT_CLEAR, PR_PREEN_OK, 0, 0, 0 },
+
        /* Pass 5 errors */
 
        /* Pass 5: Checking group summary information */

diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index b47b0c6..ef15b8c 100644 (file)
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -734,6 +734,12 @@ struct problem_context {
 /* Orphan file inode is not in use, but contains data */
 #define PR_1_ORPHAN_FILE_NOT_CLEAR             0x010090
 
+/* Inode has EA_INODE_FL set but is not a regular file */
+#define PR_1_EA_INODE_NONREG                   0x010091
+
+/* Inode references EA inode but ea_inode feature is not enabled */
+#define PR_1_EA_INODE_FEATURE                  0x010092
+
 /*
  * Pass 1b errors
  */
@@ -1061,6 +1067,9 @@ struct problem_context {
 /* Non-unique filename found, but can't rename */
 #define PR_2_NON_UNIQUE_FILE_NO_RENAME 0x020054
 
+/* EA inode referenced from directory */
+#define PR_2_EA_INODE_DIR_LINK 0x020055
+
 /*
  * Pass 3 errors
  */
@@ -1203,6 +1212,9 @@ struct problem_context {
 /* Directory ref count set to overflow but it doesn't have to be */
 #define PR_4_DIR_OVERFLOW_REF_COUNT    0x040007
 
+/* EA_INODE_FL set on normal file linked from directory hierarchy */
+#define PR_4_EA_INODE_SPURIOUS_FLAG    0x040008
+
 /*
  * Pass 5 errors
  */