public inbox for linux-erofs@ozlabs.org
 help / color / mirror / Atom feed
From: Gao Xiang <hsiangkao@linux.alibaba.com>
To: Nithurshen <nithurshen.dev@gmail.com>
Cc: linux-erofs@lists.ozlabs.org, newajay.11r@gmail.com, xiang@kernel.org
Subject: Re: [PATCH v2 experimental-tests] erofs-utils: tests: test FUSE error handling on corrupted inodes
Date: Tue, 31 Mar 2026 10:33:21 +0800	[thread overview]
Message-ID: <794b4d71-6bba-4bbd-8720-a98a82f7ddc5@linux.alibaba.com> (raw)
In-Reply-To: <20260330103051.26877-1-nithurshen.dev@gmail.com>



On 2026/3/30 18:30, Nithurshen wrote:
> This patch introduces a regression test (erofs/099) to verify that
> the FUSE daemon gracefully handles corrupted inodes without crashing
> or violating the FUSE protocol.
> 
> Recently, a bug was identified where erofs_read_inode_from_disk()
> would fail, but erofsfuse_getattr() lacked a return statement
> after sending an error reply. This caused a fall-through, sending
> a second reply via fuse_reply_attr() and triggering a libfuse
> segmentation fault.
> 
> To prevent future regressions, this test:
> 1. Creates a valid EROFS image.
> 2. Surgically corrupts the root inode (injecting random data at
>     offset 1152) while leaving the superblock intact so it mounts.
> 3. Mounts the image in the foreground to capture daemon stderr.
> 4. Runs 'stat' to trigger the inode read failure.
> 5. Evaluates the stderr log to ensure no segfaults, aborts, or
>     "multiple replies" warnings are emitted by libfuse.
> 
> Signed-off-by: Nithurshen <nithurshen.dev@gmail.com>
> ---
> Changes in v2:
> - Added $FSTYP check to ensure the test is skipped when testing the
>    kernel driver (Gao Xiang).
> - Renamed cleanup() to _cleanup() to align with standard rc teardown.
> ---
>   tests/Makefile.am   |  3 ++
>   tests/erofs/099     | 90 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/erofs/099.out |  2 +
>   3 files changed, 95 insertions(+)
>   create mode 100755 tests/erofs/099
>   create mode 100644 tests/erofs/099.out
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index e376d6a..c0f117c 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -122,6 +122,9 @@ TESTS += erofs/027
>   # 028 - test inode page cache sharing functionality
>   TESTS += erofs/028
>   
> +# 099 - test fuse error handling on truncated images
> +TESTS += erofs/099
> +
>   EXTRA_DIST = common/rc erofs
>   
>   clean-local: clean-local-check
> diff --git a/tests/erofs/099 b/tests/erofs/099
> new file mode 100755
> index 0000000..11dab4d
> --- /dev/null
> +++ b/tests/erofs/099
> @@ -0,0 +1,90 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Test FUSE daemon and kernel error handling on corrupted inodes
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$(echo $0 | awk '{print $((NF-1))"/"$NF}' FS="/")
> +
> +# get standard environment, filters and checks
> +. "${srcdir}/common/rc"
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -rf $tmp.*
> +	# Ensure we kill our background daemon if it's still alive
> +	[ -n "$fuse_pid" ] && kill -9 $fuse_pid 2>/dev/null
> +}
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +echo "QA output created by $seq"
> +
> +# Default to erofs (kernel) if FSTYP is not set
> +[ -z "$FSTYP" ] && FSTYP="erofs"
> +
> +if [ -z "$SCRATCH_DEV" ]; then
> +	SCRATCH_DEV=$tmp/erofs_$seq.img
> +	rm -f $SCRATCH_DEV
> +fi
> +
> +localdir="$tmp/$seq"
> +rm -rf $localdir
> +mkdir -p $localdir
> +
> +echo "test data" > $localdir/testfile
> +
> +_scratch_mkfs $localdir >> $seqres.full 2>&1 || _fail "failed to mkfs"

You need to disable superblock checksum using:

_scratch_mkfs -Enosbcrc $localdir >> $seqres.full 2>&1 || _fail "failed to mkfs"


> +
> +# Corrupt the root inode to force erofs_read_inode_from_disk to fail.
> +# The EROFS superblock is at offset 1024 and is 128 bytes long.
> +# The metadata (including the root inode) starts immediately after (offset 1152).

I think a better way is to use `dump.erofs` to get the NID, and use
meta_blkaddr * block_size + NID * 32 to calculate the inode offset:
for example:

$EROFSDUMP_PROG --path=/testfile $SCRATCH_DEV
...

> +# We inject 1024 bytes of random garbage starting at offset 1152. This leaves
> +# the SB intact so the mount succeeds, but guarantees the inode read will fail.
> +dd if=/dev/urandom of=$SCRATCH_DEV bs=1 seek=1152 count=1024 conv=notrunc >> $seqres.full 2>&1
> +
> +if [ "$FSTYP" = "erofsfuse" ]; then
> +	[ -z "$EROFSFUSE_PROG" ] && _notrun "erofsfuse is not available"
> +	# Run erofsfuse in the foreground to capture libfuse's internal stderr
> +	$EROFSFUSE_PROG -f $SCRATCH_DEV $SCRATCH_MNT > $tmp/fuse_err.log 2>&1 &
> +	fuse_pid=$!
> +	# Wait for the mount to establish
> +	sleep 1
> +else
> +	_require_erofs
> +	_scratch_mount >> $seqres.full 2>&1
> +fi
> +
> +# Attempt to stat the root directory. We expect this to fail with an error.
> +timeout 5 stat $SCRATCH_MNT >> $seqres.full 2>&1
> +res=$?
> +
> +if [ "$FSTYP" = "erofsfuse" ]; then
> +	# Clean up the mount
> +	umount $SCRATCH_MNT >> $seqres.full 2>&1

Can you use `_scratch_unmount`? Does that work?

> +	# Wait for the daemon to cleanly exit, or kill it if stuck
> +	kill $fuse_pid 2>/dev/null
> +	wait $fuse_pid 2>/dev/null
> +	cat $tmp/fuse_err.log >> $seqres.full
> +
> +	# Evaluate results based on captured stderr and timeout
> +	if [ $res -eq 124 ]; then
> +		_fail "stat command timed out (FUSE daemon likely hung due to double reply)"
> +	elif grep -q -i "multiple replies" $tmp/fuse_err.log; then
> +		_fail "Bug detected: libfuse reported multiple replies to request"
> +	elif grep -q -i "segmentation fault\|aborted" $tmp/fuse_err.log; then
> +		_fail "Bug detected: FUSE daemon crashed"
> +	fi
> +else
> +	# Kernel check: ensure no hang and error is returned
> +	[ $res -eq 124 ] && _fail "stat command timed out (kernel hung?)"
> +	[ $res -eq 0 ] && _fail "stat unexpectedly succeeded on a corrupted image"
> +	_scratch_unmount >> $seqres.full 2>&1
> +fi
> +
> +echo Silence is golden
> +status=0
> +exit 0
> \ No newline at end of file

Need a new line here.

Thanks,
Gao Xiang


  reply	other threads:[~2026-03-31  2:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-30  4:28 [PATCH experimental-tests] erofs-utils: tests: test FUSE error handling on corrupted inodes Nithurshen
2026-03-30  5:43 ` Gao Xiang
2026-03-30 10:30 ` [PATCH v2 " Nithurshen
2026-03-31  2:33   ` Gao Xiang [this message]
2026-04-01  7:10   ` [PATCH v3 " Nithurshen
2026-04-01  7:19     ` Gao Xiang
2026-04-01  7:55     ` [PATCH v4 " Nithurshen
2026-04-01  8:05       ` Gao Xiang
2026-04-01  8:09         ` Gao Xiang
2026-04-03  0:34       ` [PATCH v5 " Nithurshen
2026-04-07  3:01         ` [PATCH v6 " Gao Xiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=794b4d71-6bba-4bbd-8720-a98a82f7ddc5@linux.alibaba.com \
    --to=hsiangkao@linux.alibaba.com \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=newajay.11r@gmail.com \
    --cc=nithurshen.dev@gmail.com \
    --cc=xiang@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox