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, passes -l -> fsck.ext4 -a -l -C /dev/sda -> e2fsck getopt: -l consumes -C as bad_blocks_file -> journal recovery -> orphan cleanup (orphan inodes processed) -> fopen("-C", "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 --- 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 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 --- 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 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 --- 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 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 --- 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. 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 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 --- 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