Re: [PATCH] fstests: btrfs: a new test case to verify scrub and rescue=idatacsums

[Qu Wenruo <wqu@suse.com>]

在 2025/5/12 17:14, Filipe Manana 写道:
> On Mon, May 12, 2025 at 6:26 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> There is a kernel bug report that scrub will trigger a NULL pointer
>> dereference when rescue=idatacsums mount option is provided.
>>
>> Add a test case for such situation, to verify kernel can gracefully
>> reject scrub when  there is no csum tree.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   tests/btrfs/336     | 32 ++++++++++++++++++++++++++++++++
>>   tests/btrfs/336.out |  2 ++
>>   2 files changed, 34 insertions(+)
>>   create mode 100755 tests/btrfs/336
>>   create mode 100644 tests/btrfs/336.out
>>
>> diff --git a/tests/btrfs/336 b/tests/btrfs/336
>> new file mode 100755
>> index 00000000..f76a8e21
>> --- /dev/null
>> +++ b/tests/btrfs/336
>> @@ -0,0 +1,32 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2025 SUSE Linux Products GmbH. All Rights Reserved.
>> +#
>> +# FS QA Test 336
>> +#
>> +# Make sure read-only scrub won't cause NULL pointer dereference with
>> +# rescue=idatacsums mount option
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto scrub quick
>> +
>> +_fixed_by_kernel_commit 6aecd91a5c5b \
>> +       "btrfs: avoid NULL pointer dereference if no valid extent tree"
>> +
>> +_require_scratch
>> +_scratch_mkfs >> $seqres.full
>> +
>> +_try_scratch_mount "-o ro,rescue=ignoredatacsums" > /dev/null 2>&1 ||
>> +       _notrun "rescue=ignoredatacsums mount option not supported"
>> +
>> +# For unpatched kernel this will cause NULL pointer dereference and crash the kernel.
>> +# For patched kernel scrub will be gracefully rejected.
>> +$BTRFS_UTIL_PROG scrub start -Br $SCRATCH_MNT >> $seqres.full 2>&1
> 
> If the scrub is supposed to fail, as the comment says, then we should
> check that it fails.
> Right now we're ignoring whether it succeeds or fails.

Currently it indeed fails for patched kernel, but I'm not sure if it 
will keep so in the future.

E.g. we can still properly scrub metadata chunks, and for data chunks we 
may even delayed the csum tree lookup so that if we got an empty data 
block group, we just do an early exit.

Or should I do the failure check, and update the test case when the 
kernel behavior changes in the future?

Thanks,
Qu


> 
> Otherwise it looks fine, thanks.
>