| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20170626134348.1240-3-tahsin@google.com>
Date: Mon, 26 Jun 2017 06:43:39 -0700
From: Tahsin Erdogan <tahsin@...gle.com>
To: Andreas Dilger <adilger@...ger.ca>,
"Darrick J . Wong" <darrick.wong@...cle.com>,
Theodore Ts'o <tytso@....edu>, linux-ext4@...r.kernel.org
Cc: Tahsin Erdogan <tahsin@...gle.com>
Subject: [PATCH 03/12] e2fsck: ea_inode hash validation
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@...gle.com>
---
e2fsck/pass1.c | 91 +++++++++++++++++++++++++++++++++------------------
e2fsck/problem.c | 5 ---
e2fsck/problem.h | 7 ++--
lib/ext2fs/ext2fs.h | 3 ++
lib/ext2fs/ext_attr.c | 67 +++++++++++++++++++++++++++++++++++--
5 files changed, 128 insertions(+), 45 deletions(-)
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 1c68af8990b4..32152f3ec926 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -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)) {
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index dcdf9abe1abd..deffa4d66a89 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -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,
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index dad608c94fd0..8e07c10895c1 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -678,15 +678,12 @@ 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
+#define PR_1_ATTR_NO_EA_INODE_FL 0x010084
/* EA inode for parent inode does not have EXT4_EA_INODE_FL flag */
-#define PR_1_ATTR_SET_EA_INODE_FL 0x010086
+#define PR_1_ATTR_SET_EA_INODE_FL 0x010085
/*
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index f4131a640f9c..f72cbe17a8cd 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -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);
diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c
index 14d05c7e57fb..d48ab5c55270 100644
--- a/lib/ext2fs/ext_attr.c
+++ b/lib/ext2fs/ext_attr.c
@@ -25,6 +25,18 @@
#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 &&
@@ -914,9 +955,29 @@ 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 parent, child;
+
+ /* Check whether this is an old Lustre-style
+ * ea_inode reference.
+ */
+ err = ext2fs_read_inode(handle->fs, handle->ino,
+ &parent);
+ if (err)
+ return err;
+ err = ext2fs_read_inode(handle->fs,
+ entry->e_value_inum,
+ &child);
+ if (err)
+ return err;
+ if (child.i_mtime != handle->ino ||
+ child.i_generation != parent.i_generation)
+ return EXT2_ET_BAD_EA_HASH;
+ }
}
x++;
--
2.13.1.611.g7e3b11ae1-goog
Powered by blists - more mailing lists