| 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: <20210507002110.3933387-2-harshads@google.com>
Date: Thu, 6 May 2021 17:21:10 -0700
From: Harshad Shirwadkar <harshadshirwadkar@...il.com>
To: linux-ext4@...r.kernel.org
Cc: tytso@....edu, Harshad Shirwadkar <harshadshirwadkar@...il.com>
Subject: [PATCH 2/2] e2fsck: fix unaligned accesses to ext4_fc_tl struct
From: Harshad Shirwadkar <harshadshirwadkar@...il.com>
Fast commit related struct ext4_fc_tl can be unaligned on disk. So,
while accessing that we should ensure that the pointers are
aligned. This patch fixes unaligned accesses to ext4_fc_tl and also
gets rid of macros fc_for_each_tl and ext4_fc_tag_val that may result
in unaligned accesses to struct ext4_fc_tl.
Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@...il.com>
---
debugfs/logdump.c | 42 ++++++++++----------
e2fsck/journal.c | 82 +++++++++++++++++++++-------------------
lib/ext2fs/fast_commit.h | 13 -------
3 files changed, 65 insertions(+), 72 deletions(-)
diff --git a/debugfs/logdump.c b/debugfs/logdump.c
index 27e2e72d..6aee1a12 100644
--- a/debugfs/logdump.c
+++ b/debugfs/logdump.c
@@ -551,24 +551,28 @@ static inline size_t journal_super_tag_bytes(journal_superblock_t *jsb)
static void dump_fc_block(FILE *out_file, char *buf, int blocksize,
int transaction, int *fc_done, int dump_old)
{
- struct ext4_fc_tl *tl;
+ struct ext4_fc_tl tl;
struct ext4_fc_head *head;
struct ext4_fc_add_range *add_range;
struct ext4_fc_del_range *del_range;
struct ext4_fc_dentry_info *dentry_info;
struct ext4_fc_tail *tail;
struct ext3_extent *ex;
+ __u8 *cur, *val;
*fc_done = 0;
- fc_for_each_tl(buf, buf + blocksize, tl) {
- switch (le16_to_cpu(tl->fc_tag)) {
+ for (cur = (__u8 *)buf; cur < (__u8 *)buf + blocksize;
+ cur = cur + sizeof(tl) + le16_to_cpu(tl.fc_len)) {
+ memcpy(&tl, cur, sizeof(tl));
+ val = cur + sizeof(tl);
+
+ switch (le16_to_cpu(tl.fc_tag)) {
case EXT4_FC_TAG_ADD_RANGE:
- add_range =
- (struct ext4_fc_add_range *)ext4_fc_tag_val(tl);
+ add_range = (struct ext4_fc_add_range *)val;
ex = (struct ext3_extent *)add_range->fc_ex;
fprintf(out_file,
"tag %s, inode %d, lblk %u, pblk %llu, len %lu\n",
- tag2str(tl->fc_tag),
+ tag2str(tl.fc_tag),
le32_to_cpu(add_range->fc_ino),
le32_to_cpu(ex->ee_block),
le32_to_cpu(ex->ee_start) +
@@ -578,10 +582,9 @@ static void dump_fc_block(FILE *out_file, char *buf, int blocksize,
le16_to_cpu(ex->ee_len));
break;
case EXT4_FC_TAG_DEL_RANGE:
- del_range =
- (struct ext4_fc_del_range *)ext4_fc_tag_val(tl);
+ del_range = (struct ext4_fc_del_range *)val;
fprintf(out_file, "tag %s, inode %d, lblk %d, len %d\n",
- tag2str(tl->fc_tag),
+ tag2str(tl.fc_tag),
le32_to_cpu(del_range->fc_ino),
le32_to_cpu(del_range->fc_lblk),
le32_to_cpu(del_range->fc_len));
@@ -589,29 +592,26 @@ static void dump_fc_block(FILE *out_file, char *buf, int blocksize,
case EXT4_FC_TAG_LINK:
case EXT4_FC_TAG_UNLINK:
case EXT4_FC_TAG_CREAT:
- dentry_info =
- (struct ext4_fc_dentry_info *)
- ext4_fc_tag_val(tl);
+ dentry_info = (struct ext4_fc_dentry_info *)val;
fprintf(out_file,
"tag %s, parent %d, ino %d, name \"%s\"\n",
- tag2str(tl->fc_tag),
+ tag2str(tl.fc_tag),
le32_to_cpu(dentry_info->fc_parent_ino),
le32_to_cpu(dentry_info->fc_ino),
dentry_info->fc_dname);
break;
case EXT4_FC_TAG_INODE:
fprintf(out_file, "tag %s, inode %d\n",
- tag2str(tl->fc_tag),
- le32_to_cpu(((struct ext4_fc_inode *)
- ext4_fc_tag_val(tl))->fc_ino));
+ tag2str(tl.fc_tag),
+ le32_to_cpu(((struct ext4_fc_inode *)val)->fc_ino));
break;
case EXT4_FC_TAG_PAD:
- fprintf(out_file, "tag %s\n", tag2str(tl->fc_tag));
+ fprintf(out_file, "tag %s\n", tag2str(tl.fc_tag));
break;
case EXT4_FC_TAG_TAIL:
- tail = (struct ext4_fc_tail *)ext4_fc_tag_val(tl);
+ tail = (struct ext4_fc_tail *)val;
fprintf(out_file, "tag %s, tid %d\n",
- tag2str(tl->fc_tag),
+ tag2str(tl.fc_tag),
le32_to_cpu(tail->fc_tid));
if (!dump_old &&
le32_to_cpu(tail->fc_tid) < transaction) {
@@ -621,9 +621,9 @@ static void dump_fc_block(FILE *out_file, char *buf, int blocksize,
break;
case EXT4_FC_TAG_HEAD:
fprintf(out_file, "\n*** Fast Commit Area ***\n");
- head = (struct ext4_fc_head *)ext4_fc_tag_val(tl);
+ head = (struct ext4_fc_head *)val;
fprintf(out_file, "tag %s, features 0x%x, tid %d\n",
- tag2str(tl->fc_tag),
+ tag2str(tl.fc_tag),
le32_to_cpu(head->fc_features),
le32_to_cpu(head->fc_tid));
if (!dump_old &&
diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index bd0e4f31..ae3df800 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -285,9 +285,9 @@ static int ext4_fc_replay_scan(journal_t *j, struct buffer_head *bh,
struct e2fsck_fc_replay_state *state;
int ret = JBD2_FC_REPLAY_CONTINUE;
struct ext4_fc_add_range *ext;
- struct ext4_fc_tl *tl;
+ struct ext4_fc_tl tl;
struct ext4_fc_tail tail;
- __u8 *start, *end;
+ __u8 *start, *cur, *end, *val;
struct ext4_fc_head head;
struct ext2fs_extent ext2fs_ex = {0};
@@ -313,12 +313,15 @@ static int ext4_fc_replay_scan(journal_t *j, struct buffer_head *bh,
}
state->fc_replay_expected_off++;
- fc_for_each_tl(start, end, tl) {
+ for (cur = start; cur < end; cur = cur + le16_to_cpu(tl.fc_len) + sizeof(tl)) {
+ memcpy(&tl, cur, sizeof(tl));
+ val = cur + sizeof(tl);
+
jbd_debug(3, "Scan phase, tag:%s, blk %lld\n",
- tag2str(le16_to_cpu(tl->fc_tag)), bh->b_blocknr);
- switch (le16_to_cpu(tl->fc_tag)) {
+ tag2str(le16_to_cpu(tl.fc_tag)), bh->b_blocknr);
+ switch (le16_to_cpu(tl.fc_tag)) {
case EXT4_FC_TAG_ADD_RANGE:
- ext = (struct ext4_fc_add_range *)ext4_fc_tag_val(tl);
+ ext = (struct ext4_fc_add_range *)val;
ret = ext2fs_decode_extent(&ext2fs_ex, (void *)&ext->fc_ex,
sizeof(ext->fc_ex));
if (ret)
@@ -333,14 +336,14 @@ static int ext4_fc_replay_scan(journal_t *j, struct buffer_head *bh,
case EXT4_FC_TAG_INODE:
case EXT4_FC_TAG_PAD:
state->fc_cur_tag++;
- state->fc_crc = jbd2_chksum(j, state->fc_crc, tl,
- sizeof(*tl) + ext4_fc_tag_len(tl));
+ state->fc_crc = jbd2_chksum(j, state->fc_crc, cur,
+ sizeof(tl) + ext4_fc_tag_len(&tl));
break;
case EXT4_FC_TAG_TAIL:
state->fc_cur_tag++;
- memcpy(&tail, ext4_fc_tag_val(tl), sizeof(tail));
- state->fc_crc = jbd2_chksum(j, state->fc_crc, tl,
- sizeof(*tl) +
+ memcpy(&tail, val, sizeof(tail));
+ state->fc_crc = jbd2_chksum(j, state->fc_crc, cur,
+ sizeof(tl) +
offsetof(struct ext4_fc_tail,
fc_crc));
jbd_debug(1, "tail tid %d, expected %d\n",
@@ -355,7 +358,7 @@ static int ext4_fc_replay_scan(journal_t *j, struct buffer_head *bh,
state->fc_crc = 0;
break;
case EXT4_FC_TAG_HEAD:
- memcpy(&head, ext4_fc_tag_val(tl), sizeof(head));
+ memcpy(&head, val, sizeof(head));
if (le32_to_cpu(head.fc_features) &
~EXT4_FC_SUPPORTED_FEATURES) {
ret = -EOPNOTSUPP;
@@ -366,8 +369,8 @@ static int ext4_fc_replay_scan(journal_t *j, struct buffer_head *bh,
break;
}
state->fc_cur_tag++;
- state->fc_crc = jbd2_chksum(j, state->fc_crc, tl,
- sizeof(*tl) + ext4_fc_tag_len(tl));
+ state->fc_crc = jbd2_chksum(j, state->fc_crc, cur,
+ sizeof(tl) + ext4_fc_tag_len(&tl));
break;
default:
ret = state->fc_replay_num_tags ?
@@ -612,12 +615,12 @@ struct dentry_info_args {
};
static inline int tl_to_darg(struct dentry_info_args *darg,
- struct ext4_fc_tl *tl)
+ struct ext4_fc_tl *tl, __u8 *val)
{
struct ext4_fc_dentry_info fcd;
int tag = le16_to_cpu(tl->fc_tag);
- memcpy(&fcd, ext4_fc_tag_val(tl), sizeof(fcd));
+ memcpy(&fcd, val, sizeof(fcd));
darg->parent_ino = le32_to_cpu(fcd.fc_parent_ino);
darg->ino = le32_to_cpu(fcd.fc_ino);
@@ -627,7 +630,7 @@ static inline int tl_to_darg(struct dentry_info_args *darg,
if (!darg->dname)
return -ENOMEM;
memcpy(darg->dname,
- ext4_fc_tag_val(tl) + sizeof(struct ext4_fc_dentry_info),
+ val + sizeof(struct ext4_fc_dentry_info),
darg->dname_len);
darg->dname[darg->dname_len] = 0;
jbd_debug(1, "%s: %s, ino %d, parent %d\n",
@@ -638,14 +641,14 @@ static inline int tl_to_darg(struct dentry_info_args *darg,
return 0;
}
-static int ext4_fc_handle_unlink(e2fsck_t ctx, struct ext4_fc_tl *tl)
+static int ext4_fc_handle_unlink(e2fsck_t ctx, struct ext4_fc_tl *tl, __u8 *val)
{
struct ext2_inode inode;
struct dentry_info_args darg;
ext2_filsys fs = ctx->fs;
int ret;
- ret = tl_to_darg(&darg, tl);
+ ret = tl_to_darg(&darg, tl, val);
if (ret)
return ret;
ext4_fc_flush_extents(ctx, darg.ino);
@@ -657,14 +660,14 @@ static int ext4_fc_handle_unlink(e2fsck_t ctx, struct ext4_fc_tl *tl)
return ret;
}
-static int ext4_fc_handle_link_and_create(e2fsck_t ctx, struct ext4_fc_tl *tl)
+static int ext4_fc_handle_link_and_create(e2fsck_t ctx, struct ext4_fc_tl *tl, __u8 *val)
{
struct dentry_info_args darg;
ext2_filsys fs = ctx->fs;
struct ext2_inode_large inode_large;
int ret, filetype, mode;
- ret = tl_to_darg(&darg, tl);
+ ret = tl_to_darg(&darg, tl, val);
if (ret)
return ret;
ext4_fc_flush_extents(ctx, 0);
@@ -732,7 +735,7 @@ static void ext4_fc_replay_fixup_iblocks(struct ext2_inode_large *ondisk_inode,
}
}
-static int ext4_fc_handle_inode(e2fsck_t ctx, struct ext4_fc_tl *tl)
+static int ext4_fc_handle_inode(e2fsck_t ctx, __u8 *val)
{
struct e2fsck_fc_replay_state *state = &ctx->fc_replay_state;
int ino, inode_len = EXT2_GOOD_OLD_INODE_SIZE;
@@ -742,8 +745,8 @@ static int ext4_fc_handle_inode(e2fsck_t ctx, struct ext4_fc_tl *tl)
errcode_t err;
blk64_t blks;
- memcpy(&fc_ino, ext4_fc_tag_val(tl), sizeof(fc_ino));
- fc_raw_inode = ext4_fc_tag_val(tl) + sizeof(fc_ino);
+ memcpy(&fc_ino, val, sizeof(fc_ino));
+ fc_raw_inode = val + sizeof(fc_ino);
ino = le32_to_cpu(fc_ino);
if (EXT2_INODE_SIZE(ctx->fs->super) > EXT2_GOOD_OLD_INODE_SIZE)
@@ -797,13 +800,13 @@ out:
/*
* Handle add extent replay tag.
*/
-static int ext4_fc_handle_add_extent(e2fsck_t ctx, struct ext4_fc_tl *tl)
+static int ext4_fc_handle_add_extent(e2fsck_t ctx, __u8 *val)
{
struct ext2fs_extent extent;
struct ext4_fc_add_range add_range;
int ret = 0, ino;
- memcpy(&add_range, ext4_fc_tag_val(tl), sizeof(add_range));
+ memcpy(&add_range, val, sizeof(add_range));
ino = le32_to_cpu(add_range.fc_ino);
ext4_fc_flush_extents(ctx, ino);
@@ -823,13 +826,13 @@ static int ext4_fc_handle_add_extent(e2fsck_t ctx, struct ext4_fc_tl *tl)
/*
* Handle delete logical range replay tag.
*/
-static int ext4_fc_handle_del_range(e2fsck_t ctx, struct ext4_fc_tl *tl)
+static int ext4_fc_handle_del_range(e2fsck_t ctx, __u8 *val)
{
struct ext2fs_extent extent;
struct ext4_fc_del_range del_range;
int ret, ino;
- memcpy(&del_range, ext4_fc_tag_val(tl), sizeof(del_range));
+ memcpy(&del_range, val, sizeof(del_range));
ino = le32_to_cpu(del_range.fc_ino);
ext4_fc_flush_extents(ctx, ino);
@@ -854,8 +857,8 @@ static int ext4_fc_replay(journal_t *journal, struct buffer_head *bh,
e2fsck_t ctx = journal->j_fs_dev->k_ctx;
struct e2fsck_fc_replay_state *state = &ctx->fc_replay_state;
int ret = JBD2_FC_REPLAY_CONTINUE;
- struct ext4_fc_tl *tl;
- __u8 *start, *end;
+ struct ext4_fc_tl tl;
+ __u8 *start, *end, *cur, *val;
if (pass == PASS_SCAN) {
state->fc_current_pass = PASS_SCAN;
@@ -891,28 +894,31 @@ static int ext4_fc_replay(journal_t *journal, struct buffer_head *bh,
start = (__u8 *)bh->b_data;
end = (__u8 *)bh->b_data + journal->j_blocksize - 1;
- fc_for_each_tl(start, end, tl) {
+ for (cur = start; cur < end; cur = cur + le16_to_cpu(tl.fc_len) + sizeof(tl)) {
+ memcpy(&tl, cur, sizeof(tl));
+ val = cur + sizeof(tl);
+
if (state->fc_replay_num_tags == 0)
goto replay_done;
jbd_debug(3, "Replay phase processing %s tag\n",
- tag2str(le16_to_cpu(tl->fc_tag)));
+ tag2str(le16_to_cpu(tl.fc_tag)));
state->fc_replay_num_tags--;
- switch (le16_to_cpu(tl->fc_tag)) {
+ switch (le16_to_cpu(tl.fc_tag)) {
case EXT4_FC_TAG_CREAT:
case EXT4_FC_TAG_LINK:
- ret = ext4_fc_handle_link_and_create(ctx, tl);
+ ret = ext4_fc_handle_link_and_create(ctx, &tl, val);
break;
case EXT4_FC_TAG_UNLINK:
- ret = ext4_fc_handle_unlink(ctx, tl);
+ ret = ext4_fc_handle_unlink(ctx, &tl, val);
break;
case EXT4_FC_TAG_ADD_RANGE:
- ret = ext4_fc_handle_add_extent(ctx, tl);
+ ret = ext4_fc_handle_add_extent(ctx, val);
break;
case EXT4_FC_TAG_DEL_RANGE:
- ret = ext4_fc_handle_del_range(ctx, tl);
+ ret = ext4_fc_handle_del_range(ctx, val);
break;
case EXT4_FC_TAG_INODE:
- ret = ext4_fc_handle_inode(ctx, tl);
+ ret = ext4_fc_handle_inode(ctx, val);
break;
case EXT4_FC_TAG_TAIL:
ext4_fc_flush_extents(ctx, 0);
diff --git a/lib/ext2fs/fast_commit.h b/lib/ext2fs/fast_commit.h
index b83e1810..4ad38f12 100644
--- a/lib/ext2fs/fast_commit.h
+++ b/lib/ext2fs/fast_commit.h
@@ -155,13 +155,6 @@ struct ext4_fc_replay_state {
#define region_last(__region) (((__region)->lblk) + ((__region)->len) - 1)
#endif
-#define fc_for_each_tl(__start, __end, __tl) \
- for (tl = (struct ext4_fc_tl *)(__start); \
- (__u8 *)tl < (__u8 *)(__end); \
- tl = (struct ext4_fc_tl *)((__u8 *)tl + \
- sizeof(struct ext4_fc_tl) + \
- + le16_to_cpu(tl->fc_len)))
-
static inline const char *tag2str(__u16 tag)
{
switch (tag) {
@@ -194,10 +187,4 @@ static inline int ext4_fc_tag_len(struct ext4_fc_tl *tl)
return le16_to_cpu(tl->fc_len);
}
-/* Get a pointer to "value" of a tlv */
-static inline __u8 *ext4_fc_tag_val(struct ext4_fc_tl *tl)
-{
- return (__u8 *)tl + sizeof(*tl);
-}
-
#endif /* __FAST_COMMIT_H__ */
--
2.31.1.607.g51e8a6a459-goog
Powered by blists - more mailing lists