Add another falloc test entry which could hit a kernel bug
with ext4 fast_commit feature w/o below kernel commit [1].
<log>
[ 410.888496][ T2743] BUG: KASAN: use-after-free in ext4_mb_mark_bb+0x26a/0x6c0
[ 410.890432][ T2743] Read of size 8 at addr ffff888171886000 by task mount/2743
This happens when falloc -k size is huge which spans across more than
1 flex block group in ext4. This causes a bug in fast_commit replay
code which is fixed by kernel commit at [1].
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?h=dev&id=bfdc502a4a4c058bf4cbb1df0c297761d528f54d
Signed-off-by: Ritesh Harjani <[email protected]>
---
tests/generic/468 | 4 ++++
tests/generic/468.out | 2 ++
2 files changed, 6 insertions(+)
diff --git a/tests/generic/468 b/tests/generic/468
index 95752d3b..cbef9746 100755
--- a/tests/generic/468
+++ b/tests/generic/468
@@ -34,6 +34,9 @@ _scratch_mkfs >/dev/null 2>&1
_require_metadata_journaling $SCRATCH_DEV
_scratch_mount
+blocksize=4096
+fact=18
+
testfile=$SCRATCH_MNT/testfile
# check inode metadata after shutdown
@@ -85,6 +88,7 @@ for i in fsync fdatasync; do
test_falloc $i "-k " 1024
test_falloc $i "-k " 4096
test_falloc $i "-k " 104857600
+ test_falloc $i "-k " $((32768*$blocksize*$fact))
done
status=0
diff --git a/tests/generic/468.out b/tests/generic/468.out
index b3a28d5e..a09cedb8 100644
--- a/tests/generic/468.out
+++ b/tests/generic/468.out
@@ -5,9 +5,11 @@ QA output created by 468
==== falloc -k 1024 test with fsync ====
==== falloc -k 4096 test with fsync ====
==== falloc -k 104857600 test with fsync ====
+==== falloc -k 2415919104 test with fsync ====
==== falloc 1024 test with fdatasync ====
==== falloc 4096 test with fdatasync ====
==== falloc 104857600 test with fdatasync ====
==== falloc -k 1024 test with fdatasync ====
==== falloc -k 4096 test with fdatasync ====
==== falloc -k 104857600 test with fdatasync ====
+==== falloc -k 2415919104 test with fdatasync ====
--
2.31.1
On Tue, Mar 15, 2022 at 07:58:56PM +0530, Ritesh Harjani wrote:
> Add another falloc test entry which could hit a kernel bug
> with ext4 fast_commit feature w/o below kernel commit [1].
>
> <log>
> [ 410.888496][ T2743] BUG: KASAN: use-after-free in ext4_mb_mark_bb+0x26a/0x6c0
> [ 410.890432][ T2743] Read of size 8 at addr ffff888171886000 by task mount/2743
>
> This happens when falloc -k size is huge which spans across more than
> 1 flex block group in ext4. This causes a bug in fast_commit replay
> code which is fixed by kernel commit at [1].
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?h=dev&id=bfdc502a4a4c058bf4cbb1df0c297761d528f54d
>
> Signed-off-by: Ritesh Harjani <[email protected]>
> ---
> tests/generic/468 | 4 ++++
> tests/generic/468.out | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/tests/generic/468 b/tests/generic/468
> index 95752d3b..cbef9746 100755
> --- a/tests/generic/468
> +++ b/tests/generic/468
> @@ -34,6 +34,9 @@ _scratch_mkfs >/dev/null 2>&1
> _require_metadata_journaling $SCRATCH_DEV
> _scratch_mount
>
> +blocksize=4096
What happens if the file blocksize isn't 4k? Does fastcommit only
support one block size? I didn't think it has any such restriction?
> +fact=18
This needs a bit more explanation -- why 18? I think the reason is that
you need the fallocate to cross into another flexbg, and flexbgs (by
default) are 16bg long, right?
If that's the case, then don't you need to detect the flexbg size so
that this is still an effective test if someone runs fstests with
MKFS_OPTIONS='-G 32' or something?
--D
> +
> testfile=$SCRATCH_MNT/testfile
>
> # check inode metadata after shutdown
> @@ -85,6 +88,7 @@ for i in fsync fdatasync; do
> test_falloc $i "-k " 1024
> test_falloc $i "-k " 4096
> test_falloc $i "-k " 104857600
> + test_falloc $i "-k " $((32768*$blocksize*$fact))
> done
>
> status=0
> diff --git a/tests/generic/468.out b/tests/generic/468.out
> index b3a28d5e..a09cedb8 100644
> --- a/tests/generic/468.out
> +++ b/tests/generic/468.out
> @@ -5,9 +5,11 @@ QA output created by 468
> ==== falloc -k 1024 test with fsync ====
> ==== falloc -k 4096 test with fsync ====
> ==== falloc -k 104857600 test with fsync ====
> +==== falloc -k 2415919104 test with fsync ====
> ==== falloc 1024 test with fdatasync ====
> ==== falloc 4096 test with fdatasync ====
> ==== falloc 104857600 test with fdatasync ====
> ==== falloc -k 1024 test with fdatasync ====
> ==== falloc -k 4096 test with fdatasync ====
> ==== falloc -k 104857600 test with fdatasync ====
> +==== falloc -k 2415919104 test with fdatasync ====
> --
> 2.31.1
>
First of all thanks Darrick for the review.
And sorry about such a long delay in getting back to this.
On 22/03/15 09:51AM, Darrick J. Wong wrote:
> On Tue, Mar 15, 2022 at 07:58:56PM +0530, Ritesh Harjani wrote:
> > Add another falloc test entry which could hit a kernel bug
> > with ext4 fast_commit feature w/o below kernel commit [1].
> >
> > <log>
> > [ 410.888496][ T2743] BUG: KASAN: use-after-free in ext4_mb_mark_bb+0x26a/0x6c0
> > [ 410.890432][ T2743] Read of size 8 at addr ffff888171886000 by task mount/2743
> >
> > This happens when falloc -k size is huge which spans across more than
> > 1 flex block group in ext4. This causes a bug in fast_commit replay
> > code which is fixed by kernel commit at [1].
> >
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?h=dev&id=bfdc502a4a4c058bf4cbb1df0c297761d528f54d
> >
> > Signed-off-by: Ritesh Harjani <[email protected]>
> > ---
> > tests/generic/468 | 4 ++++
> > tests/generic/468.out | 2 ++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/tests/generic/468 b/tests/generic/468
> > index 95752d3b..cbef9746 100755
> > --- a/tests/generic/468
> > +++ b/tests/generic/468
> > @@ -34,6 +34,9 @@ _scratch_mkfs >/dev/null 2>&1
> > _require_metadata_journaling $SCRATCH_DEV
> > _scratch_mount
> >
> > +blocksize=4096
>
> What happens if the file blocksize isn't 4k? Does fastcommit only
> support one block size? I didn't think it has any such restriction?
It does support other block size too.
Now, for bs < 4096 it is still fine. Yes, it won't trigger for 64K bs.
But that is ok since anyway to trigger this issue with 64K, we will need a much
larger disk size that anyway folks doesn't run fstests with
$((32768*65536*18))
>
> > +fact=18
>
> This needs a bit more explanation -- why 18? I think the reason is that
> you need the fallocate to cross into another flexbg, and flexbgs (by
> default) are 16bg long, right?
That is right. Sure, I will add a comment explaining this, in the next revision.
>
> If that's the case, then don't you need to detect the flexbg size so
> that this is still an effective test if someone runs fstests with
> MKFS_OPTIONS='-G 32' or something?
We can do that. But it is rare for anyone to test only with '-G 32' and not test
with a default option. 32 might still be ok, but anything bigger then that
might still require the test to not run due to requirement of disk size.
And plus I wanted to keep the test generic enough such that it covers the
regression test with default fast_commit mkfs option.
-ritesh
>
> --D
>
> > +
> > testfile=$SCRATCH_MNT/testfile
> >
> > # check inode metadata after shutdown
> > @@ -85,6 +88,7 @@ for i in fsync fdatasync; do
> > test_falloc $i "-k " 1024
> > test_falloc $i "-k " 4096
> > test_falloc $i "-k " 104857600
> > + test_falloc $i "-k " $((32768*$blocksize*$fact))
> > done
> >
> > status=0
> > diff --git a/tests/generic/468.out b/tests/generic/468.out
> > index b3a28d5e..a09cedb8 100644
> > --- a/tests/generic/468.out
> > +++ b/tests/generic/468.out
> > @@ -5,9 +5,11 @@ QA output created by 468
> > ==== falloc -k 1024 test with fsync ====
> > ==== falloc -k 4096 test with fsync ====
> > ==== falloc -k 104857600 test with fsync ====
> > +==== falloc -k 2415919104 test with fsync ====
> > ==== falloc 1024 test with fdatasync ====
> > ==== falloc 4096 test with fdatasync ====
> > ==== falloc 104857600 test with fdatasync ====
> > ==== falloc -k 1024 test with fdatasync ====
> > ==== falloc -k 4096 test with fdatasync ====
> > ==== falloc -k 104857600 test with fdatasync ====
> > +==== falloc -k 2415919104 test with fdatasync ====
> > --
> > 2.31.1
> >