[PATCH 1/3] ext2fs: initialize handle to NULL in ext2fs_count_blks

[PATCH 1/3] ext2fs: initialize handle to NULL in ext2fs_count_blks

[Harshad Shirwadkar]

From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

Initialize the handle to NULL to ensure that in error cases,
ext2fs_free_mem can be called on it.

Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 lib/ext2fs/extent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index bde6b0f3..1a87e68b 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -1809,7 +1809,7 @@ errcode_t ext2fs_decode_extent(struct ext2fs_extent *to, void *addr, int len)
 errcode_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
 			      struct ext2_inode *inode, blk64_t *ret_count)
 {
-	ext2_extent_handle_t	handle;
+	ext2_extent_handle_t	handle = NULL;
 	struct ext2fs_extent	extent;
 	errcode_t		errcode;
 	int			i;
-- 
2.30.0.365.g02bc693789-goog

[PATCH 2/3] e2fsck: don't print errcode if the errcode is 0

[Harshad Shirwadkar]

From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

We print the error message corresponding to errcode while converting
errcode to errno. Don't do that if errcode is 0.

Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 e2fsck/journal.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index b1ca485c..922c252d 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -383,7 +383,9 @@ out_err:
 
 static int __errcode_to_errno(errcode_t err, const char *func, int line)
 {
-	fprintf(stderr, "Error \"%s\" encountered in %s at line %d\n",
+	if (err == 0)
+		return 0;
+	fprintf(stderr, "Error \"%s\" encountered in function %s at line %d\n",
 		error_message(err), func, line);
 	if (err <= 256)
 		return -err;
-- 
2.30.0.365.g02bc693789-goog

[PATCH 3/3] e2fsck: endianness fixes for fast commit replay

[Harshad Shirwadkar]

From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

There are a few places where the endianness conversion wasn't done
right. This patch fixes that. Verified that after this patch,
j_recover_fast_commit passes on big endian qemu VM.

root@debian-powerpc:~/e2fsprogs/tests# make j_recover_fast_commit
j_recover_fast_commit: : ok

Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 e2fsck/journal.c | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index 922c252d..2708942a 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -706,19 +706,19 @@ out:
 static void ext4_fc_replay_fixup_iblocks(struct ext2_inode_large *ondisk_inode,
 	struct ext2_inode_large *fc_inode)
 {
-	if (le32_to_cpu(ondisk_inode->i_flags) & EXT4_EXTENTS_FL) {
+	if (ondisk_inode->i_flags & EXT4_EXTENTS_FL) {
 		struct ext3_extent_header *eh;
 
 		eh = (struct ext3_extent_header *)(&ondisk_inode->i_block[0]);
-		if (eh->eh_magic != EXT3_EXT_MAGIC) {
+		if (le16_to_cpu(eh->eh_magic) != EXT3_EXT_MAGIC) {
 			memset(eh, 0, sizeof(*eh));
-			eh->eh_magic = EXT3_EXT_MAGIC;
+			eh->eh_magic = cpu_to_le16(EXT3_EXT_MAGIC);
 			eh->eh_max = cpu_to_le16(
 				(sizeof(ondisk_inode->i_block) -
 					sizeof(struct ext3_extent_header)) /
-					sizeof(struct ext3_extent));
+				sizeof(struct ext3_extent));
 		}
-	} else if (le32_to_cpu(ondisk_inode->i_flags) & EXT4_INLINE_DATA_FL) {
+	} else if (ondisk_inode->i_flags & EXT4_INLINE_DATA_FL) {
 		memcpy(ondisk_inode->i_block, fc_inode->i_block,
 			sizeof(fc_inode->i_block));
 	}
@@ -728,34 +728,41 @@ static int ext4_fc_handle_inode(e2fsck_t ctx, struct ext4_fc_tl *tl)
 {
 	struct e2fsck_fc_replay_state *state = &ctx->fc_replay_state;
 	int ino, inode_len = EXT2_GOOD_OLD_INODE_SIZE;
-	struct ext2_inode_large *inode = NULL;
-	struct ext4_fc_inode *fc_inode;
+	struct ext2_inode_large *inode = NULL, *fc_inode = NULL;
+	struct ext4_fc_inode *fc_inode_val;
 	errcode_t err;
 	blk64_t blks;
 
-	fc_inode = (struct ext4_fc_inode *)ext4_fc_tag_val(tl);
-	ino = le32_to_cpu(fc_inode->fc_ino);
+	fc_inode_val = (struct ext4_fc_inode *)ext4_fc_tag_val(tl);
+	ino = le32_to_cpu(fc_inode_val->fc_ino);
 
 	if (EXT2_INODE_SIZE(ctx->fs->super) > EXT2_GOOD_OLD_INODE_SIZE)
 		inode_len += ext2fs_le16_to_cpu(
-			((struct ext2_inode_large *)fc_inode->fc_raw_inode)
+			((struct ext2_inode_large *)fc_inode_val->fc_raw_inode)
 				->i_extra_isize);
 	err = ext2fs_get_mem(inode_len, &inode);
 	if (err)
-		return errcode_to_errno(err);
+		goto out;
+	err = ext2fs_get_mem(inode_len, &fc_inode);
+	if (err)
+		goto out;
 	ext4_fc_flush_extents(ctx, ino);
 
 	err = ext2fs_read_inode_full(ctx->fs, ino, (struct ext2_inode *)inode,
 					inode_len);
 	if (err)
 		goto out;
-	memcpy(inode, fc_inode->fc_raw_inode,
-		offsetof(struct ext2_inode_large, i_block));
-	memcpy(&inode->i_generation,
-		&((struct ext2_inode_large *)(fc_inode->fc_raw_inode))->i_generation,
+#ifdef WORDS_BIGENDIAN
+	ext2fs_swap_inode_full(ctx->fs, fc_inode,
+			       (struct ext2_inode_large *)fc_inode_val->fc_raw_inode,
+			       0, sizeof(*inode));
+#else
+	memcpy(fc_inode, fc_inode_val->fc_raw_inode, inode_len);
+#endif
+	memcpy(inode, fc_inode, offsetof(struct ext2_inode_large, i_block));
+	memcpy(&inode->i_generation, &fc_inode->i_generation,
 		inode_len - offsetof(struct ext2_inode_large, i_generation));
-	ext4_fc_replay_fixup_iblocks(inode,
-		(struct ext2_inode_large *)fc_inode->fc_raw_inode);
+	ext4_fc_replay_fixup_iblocks(inode, fc_inode);
 	err = ext2fs_count_blocks(ctx->fs, ino, EXT2_INODE(inode), &blks);
 	if (err)
 		goto out;
@@ -774,6 +781,7 @@ static int ext4_fc_handle_inode(e2fsck_t ctx, struct ext4_fc_tl *tl)
 
 out:
 	ext2fs_free_mem(&inode);
+	ext2fs_free_mem(&fc_inode);
 	return errcode_to_errno(err);
 }
 
@@ -819,7 +827,7 @@ static int ext4_fc_handle_del_range(e2fsck_t ctx, struct ext4_fc_tl *tl)
 
 	memset(&extent, 0, sizeof(extent));
 	extent.e_lblk = ext2fs_le32_to_cpu(del_range->fc_lblk);
-	extent.e_len = ext2fs_le16_to_cpu(del_range->fc_len);
+	extent.e_len = ext2fs_le32_to_cpu(del_range->fc_len);
 	ret = ext4_fc_read_extents(ctx, ino);
 	if (ret)
 		return ret;
-- 
2.30.0.365.g02bc693789-goog

Re: [PATCH 1/3] ext2fs: initialize handle to NULL in ext2fs_count_blks

[Theodore Ts'o]

On Thu, Feb 04, 2021 at 03:35:59PM -0800, Harshad Shirwadkar wrote:
> From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> 
> Initialize the handle to NULL to ensure that in error cases,
> ext2fs_free_mem can be called on it.
> 
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

Thanks, applied.

					- Ted

Re: [PATCH 2/3] e2fsck: don't print errcode if the errcode is 0

[Theodore Ts'o]

On Thu, Feb 04, 2021 at 03:36:00PM -0800, Harshad Shirwadkar wrote:
> From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> 
> We print the error message corresponding to errcode while converting
> errcode to errno. Don't do that if errcode is 0.
> 
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

It looks like this change is already in commit 63b7192cae60 ("e2fsck:
add fc replay for link, unlink, creat tags") which is already in the
upstream repo?

						- Ted

Re: [PATCH 3/3] e2fsck: endianness fixes for fast commit replay

[Theodore Ts'o]

On Thu, Feb 04, 2021 at 03:36:01PM -0800, Harshad Shirwadkar wrote:
> From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> 
> There are a few places where the endianness conversion wasn't done
> right. This patch fixes that. Verified that after this patch,
> j_recover_fast_commit passes on big endian qemu VM.
> 
> root@debian-powerpc:~/e2fsprogs/tests# make j_recover_fast_commit
> j_recover_fast_commit: : ok
> 
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>

Thanks, applied.

					- Ted