e2fsck: Fix potential data corruptor bug in journal recovery

While adding support for journal checksums into e2fsck, I found the following rather embarassing bug. It can potentially cause data/fileysystem corruption, although tripping it would be quite rare. Still, Enterprise distro's might consider this worthy of a bugfix update --- that would have to be their call. I've checked this into e2fsprogs's maint branch and will probably release e2fsprogs 1.40.10 as a result of finding this bug, and cc'ed people that I know are involved with Red Hat, SuSE and Ubuntu as a heads up.

If you want the minimalistic (one character!) patch to fix this bug, here it is:

index 22e79a5..f7fd7b6 100644
@@ -473,7 +473,7 @@ static int do_one_pass(journal_t *journal,
 					memcpy(nbh->b_data, obh->b_data,
 							journal->j_blocksize);
 					if (flags & JFS_FLAG_ESCAPE) {
-						*((unsigned int *)bh->b_data) =
+						*((unsigned int *)nbh->b_data) =
 							htonl(JFS_MAGIC_NUMBER);
 					}
 

- Ted

From: Theodore Ts'o <[email protected]>
Date: Tue, 20 May 2008 14:51:14 -0400
Subject: [PATCH] e2fsck: Fix potential data corruptor bug in journal recovery

While synchronizing e2fsck's recovery.c with the latest 2.6 kernel sources, I discovered a serious bug that apparently had been fixed in the kernel sometime between Deceber 2003 and April 2005, but which had not been carried over to e2fsprogs. Specifically, when blocks whose first 4 bytes are JFS_MAGIC_NUMBER (0xc03b3998) are written into the journal, the first 4 bytes zero'ed out. A one character typo meant that when the blocks were replayed by e2fsck, the JFS_MAGIC_NUMBER would not be restored.

Oops.

Fortunately, it is *highly* unlikely that ext4 metadata blocks will contain that magic number in the first four bytes, and data=journalled is a relatively rarely used.

This commit fixes this bug, as well as updating e2fsck's recovery.c to be in sync with 2.6.25.

---
 e2fsck/jfs_user.h |    4 +
 e2fsck/recovery.c |  171 +++++++++++++++++++++++++++--------------------------
 2 files changed, 92 insertions(+), 83 deletions(-)

index c38def3..9da5a16 100644
@@ -59,6 +59,10 @@ typedef struct {
 #define kmalloc(len,flags) malloc(len)
 #define kfree(p) free(p)
 
+#define cond_resched()	do { } while (0)
+
+typedef unsigned int __be32;
+
 /*
  * We use the standard libext2fs portability tricks for inline
  * functions.  

1.5.4.1.144.gdfee-dirty