[PATCH e2fsprogs 0/5] fsck: fix stale "bad orphan inode" after fsck runs

[PATCH e2fsprogs 0/5] fsck: fix stale "bad orphan inode" after fsck runs

[Baokun Li]

systemd-fsck invokes the fsck wrapper with options (-l, -C <fd>) that
the e2fsprogs wrapper mis-parses and leaks to e2fsck, where they get
eaten as a bad_blocks_file. e2fsck then aborts after orphan cleanup but
before flushing the superblock, leaving s_last_orphan stale on disk and
the kernel reporting "bad orphan inode" on next mount.

This series fixes both wrapper parsing bugs (consume -l; parse a
separate "-C <fd>"), and, as defense-in-depth, flushes the superblock
right after orphan cleanup so an early abort can no longer corrupt the
on-disk orphan state. Two regression tests are added. See the
individual patches for details.

Comments and review are welcome.

Thanks,
Baokun

Baokun Li (5):
  fsck: consume -l option instead of passing to e2fsck
  fsck: fix -C fd option parsing when fd is a separate argument
  e2fsck: flush superblock immediately after orphan cleanup
  tests: add fsck option-parsing regression test
  tests: add orphan-cleanup flush regression test

 e2fsck/super.c                      | 11 ++++++
 misc/fsck.c                         |  5 ++-
 tests/f_fsck_opt_parse/expect.1     |  2 +
 tests/f_fsck_opt_parse/name         |  1 +
 tests/f_fsck_opt_parse/script       | 61 +++++++++++++++++++++++++++++
 tests/f_orphan_flush_abort/expect.1 |  7 ++++
 tests/f_orphan_flush_abort/name     |  1 +
 tests/f_orphan_flush_abort/script   | 61 +++++++++++++++++++++++++++++
 tests/test_config                   |  2 +
 9 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 tests/f_fsck_opt_parse/expect.1
 create mode 100644 tests/f_fsck_opt_parse/name
 create mode 100644 tests/f_fsck_opt_parse/script
 create mode 100644 tests/f_orphan_flush_abort/expect.1
 create mode 100644 tests/f_orphan_flush_abort/name
 create mode 100644 tests/f_orphan_flush_abort/script

-- 
2.43.7

[PATCH e2fsprogs 1/5] fsck: consume -l option instead of passing to e2fsck

[Baokun Li]

The -l (lockdisk) option was introduced by util-linux in commit
dd0bd943f9 ("fsck: add support for whole-disk locking (-l option)",
2010-10-26, first released in v2.19). systemd-fsck passes -l to
/sbin/fsck for parallel fsck support since commit 87c3e6c ("fsck:
pass -l to fsck").

Currently the e2fsprogs fsck wrapper (misc/fsck.c, installed via
make install) does not recognize -l and passes it through to e2fsck.
Since e2fsck's getopt string declares "l:" (requires argument), it
consumes the next argument as the bad_blocks_file parameter. This
triggers a chain of failures:

  systemd-fsck -> fsck -a -T -l -M -C6 /dev/sda
    -> wrapper consumes -a,-T,-M,-C6, adds -C<fd>, passes -l
      -> fsck.ext4 -a -l -C<fd> /dev/sda
        -> e2fsck getopt: -l consumes -C<fd> as bad_blocks_file
          -> journal recovery
          -> orphan cleanup (orphan inodes processed)
          -> fopen("-C<fd>", "r") fails -> E2F_FLAG_ABORT
            -> superblock not flushed (s_last_orphan still non-zero)
              -> kernel: "bad orphan inode" on next mount

This is because the e2fsprogs wrapper lacks a case for -l in its
option parsing, unlike the util-linux wrapper which handles it
correctly (lockdisk = 1).

Fix by adding case 'l': break; to consume the option in the wrapper,
matching util-linux's behavior. Users can still pass -l to e2fsck
via the "--" separator (e.g., "fsck -- -l badblocks_file /dev/sda")
or by invoking e2fsck directly.

Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 misc/fsck.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/misc/fsck.c b/misc/fsck.c
index 44577590bca5..cb935e1b4b71 100644
--- a/misc/fsck.c
+++ b/misc/fsck.c
@@ -1212,6 +1212,9 @@ static void PRS(int argc, char *argv[])
 			case 'M':
 				ignore_mounted++;
 				break;
+			case 'l':
+				/* lockdisk option from util-linux fsck, consumed by wrapper */
+				break;
 			case 'P':
 				parallel_root++;
 				break;
-- 
2.43.7

[PATCH e2fsprogs 2/5] fsck: fix -C fd option parsing when fd is a separate argument

[Baokun Li]

The e2fsprogs fsck wrapper (misc/fsck.c, installed via make install)
supports the -C option for progress reporting. The fd can be passed
either attached (-C6) or separated (-C 6).

Currently the wrapper handles "-C6" correctly, but fails to handle
"-C 6" properly:

  # fsck -h | head -n1
  fsck 1.47.4 (6-Mar-2025)
  # fsck -C6 -N /dev/sda
  [/sbin/fsck.ext4 (1) -- /dev/sda] fsck.ext4 -C6 /dev/sda
  # fsck -C 6 -N /dev/sda
  [/sbin/fsck.ext4 (1) -- /dev/sda] fsck.ext4 6 -C0 /dev/sda

When "-C 6" is passed, it is parsed as "-C0" with "6" leaked as a
stray argument. This is because in PRS(), the separate-argument branch
uses argv[i] (which is "-C") instead of argv[i+1] (which is "6")
to parse the fd value. string_to_int("-C") returns -1, so progress_fd
stays at 0 and the "6" is never consumed. Later the wrapper generates
"-C0" from progress_fd=0 and the stray "6" is passed before the device.

Fix by using argv[i+1] to correctly parse the separate fd argument.

Note: this is the e2fsprogs fsck wrapper (misc/fsck.c), not the
util-linux fsck wrapper (disk-utils/fsck.c) which handles this
correctly.

Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 misc/fsck.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/misc/fsck.c b/misc/fsck.c
index cb935e1b4b71..318ecfcb4102 100644
--- a/misc/fsck.c
+++ b/misc/fsck.c
@@ -1188,7 +1188,7 @@ static void PRS(int argc, char *argv[])
 						goto next_arg;
 				} else if (argc > i + 1 &&
 					   argv[i + 1][0] != '-') {
-					progress_fd = string_to_int(argv[i]);
+					progress_fd = string_to_int(argv[i+1]);
 					if (progress_fd < 0)
 						progress_fd = 0;
 					else {
-- 
2.43.7

[PATCH e2fsprogs 3/5] e2fsck: flush superblock immediately after orphan cleanup

[Baokun Li]

When e2fsck aborts after processing orphan inodes but before the
final superblock flush, the kernel reports "bad orphan inode" errors
on next mount. This happens when e2fsck encounters fatal errors
(e.g., bad_blocks_file open failures) after orphan cleanup completes.

The root cause is a timing issue in release_orphan_inodes(). For each
orphan it rewrites the inode (updating i_dtime from the next-orphan
pointer to a timestamp, and for a delete also clearing the inode bitmap
and freeing blocks) and zeroes s_last_orphan in memory. The inode
writes go through the io cache and may be written back to disk at any
point as the cache fills during later passes, but the superblock update
(s_last_orphan = 0) is only marked dirty and is not written until the
end of the fsck run. If e2fsck aborts in between, the on-disk inode has
already been rewritten while the on-disk superblock still points to it
via the stale s_last_orphan.

When the kernel mounts such a filesystem, it follows the stale
s_last_orphan pointer and finds an inode whose i_dtime field now
contains a timestamp instead of a valid inode number. The kernel
interprets this as an invalid next-orphan pointer and reports the
error.

Fix this by flushing the superblock and bitmaps via ext2fs_flush()
(which also flushes the io cache) right after a successful orphan
cleanup, keeping on-disk state consistent regardless of what happens
later. We only flush on success; if release_orphan_inodes() fails, the
filesystem is already marked as needing a full fsck.

Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 e2fsck/super.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/e2fsck/super.c b/e2fsck/super.c
index 22af1b72684d..e8e4239fc037 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -570,6 +570,17 @@ static int release_orphan_inodes(e2fsck_t ctx)
 
 	ext2fs_free_mem(&block_buf);
 	pctx.errcode = e2fsck_write_all_quotas(ctx);
+	if (pctx.errcode)
+		goto err;
+	/*
+	 * Orphan cleanup is a consistency point: force the superblock and
+	 * bitmaps to disk now.  Otherwise a later abnormal exit could leave
+	 * a stale s_last_orphan on disk while the orphan inodes it points to
+	 * have already been written back, which the kernel rejects as a
+	 * "bad orphan inode".  On error, fall through so the caller marks
+	 * the filesystem as needing a full fsck.
+	 */
+	pctx.errcode = ext2fs_flush(fs);
 	if (pctx.errcode)
 		goto err;
 	return 0;
-- 
2.43.7

[PATCH e2fsprogs 4/5] tests: add fsck option-parsing regression test

[Baokun Li]

Add f_fsck_opt_parse, which runs the fsck wrapper in dry-run mode
(-N) with -V so it echoes the exact argv it builds for the checker,
and asserts the wrapper:

  - consumes the util-linux "-l" (lockdisk) option instead of leaking
    it to e2fsck, where -l takes a bad-blocks-file argument and would
    swallow the following option; and
  - parses "-C 2" (progress fd as a separate argument) into "-C2"
    instead of dropping it and leaking "2" as a stray device name.

Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 tests/f_fsck_opt_parse/expect.1 |  2 ++
 tests/f_fsck_opt_parse/name     |  1 +
 tests/f_fsck_opt_parse/script   | 61 +++++++++++++++++++++++++++++++++
 tests/test_config               |  2 ++
 4 files changed, 66 insertions(+)
 create mode 100644 tests/f_fsck_opt_parse/expect.1
 create mode 100644 tests/f_fsck_opt_parse/name
 create mode 100644 tests/f_fsck_opt_parse/script

diff --git a/tests/f_fsck_opt_parse/expect.1 b/tests/f_fsck_opt_parse/expect.1
new file mode 100644
index 000000000000..10b716cd16a2
--- /dev/null
+++ b/tests/f_fsck_opt_parse/expect.1
@@ -0,0 +1,2 @@
+[fsck.ext4 (1) -- test.img] fsck.ext4 -f -C2 test.img 
+Exit status is 0
diff --git a/tests/f_fsck_opt_parse/name b/tests/f_fsck_opt_parse/name
new file mode 100644
index 000000000000..37135f686288
--- /dev/null
+++ b/tests/f_fsck_opt_parse/name
@@ -0,0 +1 @@
+fsck wrapper consumes -l and parses "-C fd" as a separate argument
diff --git a/tests/f_fsck_opt_parse/script b/tests/f_fsck_opt_parse/script
new file mode 100644
index 000000000000..92597ac11754
--- /dev/null
+++ b/tests/f_fsck_opt_parse/script
@@ -0,0 +1,61 @@
+if ! test -x $FSCK_WRAPPER_EXE; then
+	echo "$test_name: $test_description: skipped (no fsck wrapper)"
+	return 0
+fi
+
+OUT=$test_name.log
+
+$MKE2FS -Fq -t ext4 -o Linux $TMPFILE 4M > /dev/null 2>&1
+
+# The fsck wrapper resolves fsck.<type> via a fixed search path
+# (/sbin:...:$PATH).  Provide our own fsck.ext4 on $PATH as a fallback so a
+# checker is always found even in a clean build tree with none installed.  A
+# system /sbin/fsck.ext4, if present, is searched first and used instead, but
+# either way the printed path is normalized by filter.sed below.
+WRAPPER_DIR="$test_name.fsckdir"
+mkdir -p $WRAPPER_DIR
+ln -sf "$(cd ../e2fsck && pwd)/e2fsck" $WRAPPER_DIR/fsck.ext4
+SAVED_PATH="$PATH"
+export PATH="$WRAPPER_DIR:$PATH"
+
+# Suppress "WARNING: couldn't open /etc/fstab" from the fsck wrapper.
+SAVED_FSTAB_FILE="${FSTAB_FILE-unset}"
+export FSTAB_FILE=/dev/null
+
+cp /dev/null $OUT
+
+# Dry run (-N) so no checker is executed; -V echoes the exact argv the
+# wrapper builds for the checker, which is what we are asserting.  -T
+# suppresses the version banner so the output is stable across releases.
+#
+#   -l       lockdisk option from util-linux; the wrapper must consume it
+#            rather than leak it to e2fsck (where -l takes a bad-blocks-file
+#            argument and would swallow the following option).
+#   -C 2     progress fd given as a separate argument; the wrapper must read
+#            the "2" and emit "-C2", not drop it and leak "2" as a device.
+$FSCK_WRAPPER -N -T -V -f -l -C 2 $TMPFILE > $OUT.new 2>&1
+status=$?
+echo "Exit status is $status" >> $OUT.new
+sed -f $cmd_dir/filter.sed \
+	-e "s|$TMPFILE|test.img|g" \
+	-e "s|\[[^]]*fsck\.ext4|[fsck.ext4|g" $OUT.new > $OUT
+rm -f $OUT.new
+
+cmp -s $OUT $test_dir/expect.1
+status=$?
+if [ "$status" -eq 0 ]; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $test_dir/expect.1 $OUT > $test_name.failed
+fi
+
+export PATH="$SAVED_PATH"
+if [ "$SAVED_FSTAB_FILE" = "unset" ]; then
+	unset FSTAB_FILE
+else
+	export FSTAB_FILE="$SAVED_FSTAB_FILE"
+fi
+rm -rf $WRAPPER_DIR
+unset OUT SAVED_PATH SAVED_FSTAB_FILE
diff --git a/tests/test_config b/tests/test_config
index 9dc762ce5a88..cb2654d59266 100644
--- a/tests/test_config
+++ b/tests/test_config
@@ -13,6 +13,8 @@ E2IMAGE="$USE_VALGRIND ../misc/e2image"
 E2IMAGE_EXE="../misc/e2image"
 DEBUGFS="$USE_VALGRIND ../debugfs/debugfs"
 DEBUGFS_EXE="../debugfs/debugfs"
+FSCK_WRAPPER="$USE_VALGRIND ../misc/fsck"
+FSCK_WRAPPER_EXE="../misc/fsck"
 TEST_BITS="test_data.tmp"
 RESIZE2FS_EXE="../resize/resize2fs"
 RESIZE2FS="$USE_VALGRIND $RESIZE2FS_EXE"
-- 
2.43.7

[PATCH e2fsprogs 5/5] tests: add orphan-cleanup flush regression test

[Baokun Li]

Add f_orphan_flush_abort, which drives e2fsck directly:

1. Build an ext4 image whose inode 12 is an orphan (links_count 0,
   s_last_orphan = 12).
2. Run e2fsck with a nonexistent bad-blocks file so that orphan
   cleanup runs first and then read_bad_blocks_file aborts the run
   via exit() before the normal superblock write-back.
3. Check that s_last_orphan is 0 on disk afterwards.

Without the flush after orphan cleanup, s_last_orphan stays stale on
disk (still 12) while the orphan inode has already been written back,
which the kernel rejects as a "bad orphan inode" on next mount.

Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 tests/f_orphan_flush_abort/expect.1 |  7 ++++
 tests/f_orphan_flush_abort/name     |  1 +
 tests/f_orphan_flush_abort/script   | 61 +++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+)
 create mode 100644 tests/f_orphan_flush_abort/expect.1
 create mode 100644 tests/f_orphan_flush_abort/name
 create mode 100644 tests/f_orphan_flush_abort/script

diff --git a/tests/f_orphan_flush_abort/expect.1 b/tests/f_orphan_flush_abort/expect.1
new file mode 100644
index 000000000000..0a6d09f16e36
--- /dev/null
+++ b/tests/f_orphan_flush_abort/expect.1
@@ -0,0 +1,7 @@
+Clearing orphaned inode 12 (uid=0, gid=0, mode=0100644, size=8192)
+read_bad_blocks_file: No such file or directory while trying to open test.img
+
+test.img: ***** FILE SYSTEM WAS MODIFIED *****
+Exit status is 9
+Orphan before e2fsck: 12
+Orphan after e2fsck: 0 (cleared)
diff --git a/tests/f_orphan_flush_abort/name b/tests/f_orphan_flush_abort/name
new file mode 100644
index 000000000000..c9292546c6dd
--- /dev/null
+++ b/tests/f_orphan_flush_abort/name
@@ -0,0 +1 @@
+orphan cleanup superblock flush survives a later abort
diff --git a/tests/f_orphan_flush_abort/script b/tests/f_orphan_flush_abort/script
new file mode 100644
index 000000000000..9c18011b40db
--- /dev/null
+++ b/tests/f_orphan_flush_abort/script
@@ -0,0 +1,61 @@
+if ! test -x $DEBUGFS_EXE; then
+	echo "$test_name: $test_description: skipped (no debugfs)"
+	return 0
+fi
+
+TEST_DATA="$test_name.tmp"
+OUT=$test_name.log
+
+dd if=/dev/zero of=$TEST_DATA bs=8k count=1 > /dev/null 2>&1
+
+$MKE2FS -Fq -t ext4 -o Linux $TMPFILE 4M > /dev/null 2>&1
+
+# Create an orphan inode: testfile becomes inode 12 (the first regular inode
+# on a fresh fs), drop its link count to 0 and point s_last_orphan at it so
+# e2fsck will run orphan cleanup.
+$DEBUGFS -w $TMPFILE << EOF > /dev/null 2>&1
+write $TEST_DATA testfile
+set_inode_field testfile links_count 0
+set_super_value last_orphan 12
+unlink testfile
+quit
+EOF
+
+ORPHAN_BEFORE=$($DUMPE2FS $TMPFILE 2>&1 | grep -E "First orphan inode" | awk '{print $NF}')
+
+cp /dev/null $OUT
+
+# Run e2fsck with a nonexistent bad blocks file.  Orphan cleanup runs first
+# and clears s_last_orphan, then read_bad_blocks_file fails and e2fsck aborts
+# via exit() before the normal superblock write-back.  The fix flushes the
+# superblock right after orphan cleanup, so s_last_orphan is 0 on disk even
+# though the run aborted; without it the disk keeps a stale s_last_orphan
+# while the orphan inode has already been written back, which the kernel
+# rejects as a "bad orphan inode" on next mount.
+$FSCK -yf -l /nonexistent/bad_blocks_file $TMPFILE > $OUT.new 2>&1
+status=$?
+echo "Exit status is $status" >> $OUT.new
+sed -f $cmd_dir/filter.sed -e "s|$TMPFILE|test.img|g" $OUT.new > $OUT
+rm -f $OUT.new
+
+ORPHAN_AFTER=$($DUMPE2FS $TMPFILE 2>&1 | grep -E "First orphan inode" | awk '{print $NF}')
+
+echo "Orphan before e2fsck: $ORPHAN_BEFORE" >> $OUT
+if [ -z "$ORPHAN_AFTER" ]; then
+	echo "Orphan after e2fsck: 0 (cleared)" >> $OUT
+else
+	echo "Orphan after e2fsck: $ORPHAN_AFTER (BUG: should be 0!)" >> $OUT
+fi
+
+cmp -s $OUT $test_dir/expect.1
+status=$?
+if [ "$status" -eq 0 ]; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $test_dir/expect.1 $OUT > $test_name.failed
+fi
+
+rm -f $TEST_DATA
+unset OUT TEST_DATA
-- 
2.43.7