On Wed, Dec 14, 2022 at 03:12:01PM -0800, Boris Burkov wrote:
If we create a file that has an inline extent followed by a prealloc
extent, then attempt to use the logical to inode ioctl on the prealloc
extent, but in the overwritten range, backref resolution will process
the inline extent. Depending on the leaf eb layout, this can panic.
Add a new test for this condition. In the long run, we can add spew when
we read out-of-bounds fields of inline extent items and simplify this
test to look for dmesg warnings rather than trying to force a fairly
fragile panic (dependent on non-standardized details of leaf layout).
The test causes a kernel panic unless:
btrfs: fix logical_ino ioctl panic
Hi,
Thanks for this new case.
Is this patch merged, does it has a commit id? BTW, please mark this
known issue by _fixed_by_kernel_commit in case source code.
is applied to the kernel.
Signed-off-by: Boris Burkov <boris@xxxxxx>
---
tests/btrfs/279 | 95 +++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/279.out | 2 +
2 files changed, 97 insertions(+)
create mode 100755 tests/btrfs/279
create mode 100644 tests/btrfs/279.out
diff --git a/tests/btrfs/279 b/tests/btrfs/279
new file mode 100755
index 00000000..ef77f84b
--- /dev/null
+++ b/tests/btrfs/279
btrfs/279 was just token last weekend, please help to rebase to the latest
fstests for-next branch, or change the 279 to a big enough number which won't
cause conflict (I'll give it a proper number when merge it).
@@ -0,0 +1,95 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Meta Platforms, Inc. All Rights Reserved.
+#
+# FS QA Test 279
+#
+# Given a file with extents:
+# [0 : 4096) (inline)
+# [4096 : N] (prealloc)
+# if a user uses the ioctl BTRFS_IOC_LOGICAL_INO[_V2] asking for the file of the
+# non-inline extent, it results in reading the offset field of the inline
+# extent, which is meaningless (it is full of user data..). If we are
+# particularly lucky, it can be past the end of the extent buffer, resulting in
+# a crash.
+#
+. ./common/preamble
+_begin_fstest auto quick
+
+# Import common functions.
+. ./common/filter
+. ./common/dmlogwrites
Does this case really use helpers in above two included files?
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs btrfs
+_require_scratch
+_require_xfs_io_command "falloc"
Add this case to prealloc or preallocrw group
+_require_xfs_io_command "fsync"
+_require_xfs_io_command "pwrite"
+
+dump_tree() {
+ $BTRFS_UTIL_PROG inspect-internal dump-tree $SCRATCH_DEV
_require_btrfs_command inspect-internal dump-tree
+}
+
+get_extent_data() {
+ local ino=$1
+ dump_tree $SCRATCH_DEV | grep -A4 "($ino EXTENT_DATA "
+}
+
+get_prealloc_offset() {
+ local ino=$1
+ get_extent_data $ino | grep "disk byte" | awk '{print $5}'
+}
+
+# This test needs to create conditions s.t. the special inode's inline extent
+# is the first item in a leaf. Therefore, fix a leaf size and add
+# items that are otherwise not necessary to reproduce the inline-prealloc
+# condition to get to such a state.
+#
+# Roughly, the idea for getting the right item fill is to:
+# 1. create an extra file with a variable sized inline extent item
+# 2. create our evil file that will cause the panic
+# 3. create a whole bunch of files to create a bunch of dir/index items
+# 4. size the variable extent item s.t. the evil extent item is item 0 in the
+# next leaf
+#
+# We do it in this somewhat convoluted way because the dir and index items all
+# come before any inode, inode_ref, or extent_data items. So we can predictably
+# create a bunch of them, then sneak in a funny sized extent to round out the
+# difference.
+
+_scratch_mkfs "--nodesize 16k" >/dev/null
We recommend calling _fail at here if _scratch_mkfs fails with a *specified*
option.
_scratch_mkfs "--nodesize 16k" >>$seqres.full || _fail "mkfs failed"
+_scratch_mount
+
+f=$SCRATCH_MNT/f
+
+# the variable extra "leaf padding" file
+$XFS_IO_PROG -fc "pwrite -q 0 700" $f.pad
+
+# the evil file with an inline extent followed by a prealloc extent
+# created by falloc with keep-size, then two non-truncating writes to the front
+touch $f.evil
Not sure if you reall use this touch command as you've used "-f" option for
xfs_io.
+$XFS_IO_PROG -fc "falloc -k 0 1m" $f.evil
+$XFS_IO_PROG -fc fsync $f.evil
+ino=$(stat -c '%i' $f.evil)
+logical=$(get_prealloc_offset $ino)
+$XFS_IO_PROG -fc "pwrite -q 0 23" $f.evil
+$XFS_IO_PROG -fc fsync $f.evil
+$XFS_IO_PROG -fc "pwrite -q 0 23" $f.evil
+$XFS_IO_PROG -fc fsync $f.evil
xfs_io commands can be combined in one line, to make the code looks clear (except
you hope the fd closed after each command done) E.g:
$XFS_IO_PROG -fc "falloc -k 0 1m" -c fsync $f.evil
ino=$(stat -c '%i' $f.evil)
logical=$(get_prealloc_offset $ino)
$XFS_IO_PROG -c "pwrite -q 0 23" -c fsync \
-c "pwrite -q 0 23" -c fsync \
$f.evil
+sync
+
+# a bunch of inodes to stuff dir items in front of the extent items
+for i in $(seq 122); do
+ $XFS_IO_PROG -fc "pwrite -q 0 8192" $f.$i
+done
+sync
+
+btrfs inspect-internal logical-resolve $logical $SCRATCH_MNT | _filter_scratch
^^
$BTRFS_UTIL_PROG
_require_btrfs_command inspect-internal logical-resolve
The .out file only has "Silence is golden", what's this _filter_scratch used for?
+_scratch_unmount
Is the ending _scratch_unmount useful?
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/btrfs/279.out b/tests/btrfs/279.out
new file mode 100644
index 00000000..c5906093
--- /dev/null
+++ b/tests/btrfs/279.out
@@ -0,0 +1,2 @@
+QA output created by 279
+Silence is golden
--
2.38.1
