* Re: [PATCH v2] ext4: test reserved GID block allocation
2026-11-07 19:30 ` [PATCH v2] ext4: test reserved GID block allocation Sahaj Chaudhari
@ 2026-11-07 19:41 ` sashiko-bot
2026-11-09 13:08 ` Jan Kara
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-07 19:41 UTC (permalink / raw)
To: Sahaj Chaudhari; +Cc: linux-ext4, tytso
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] ext4/309: hardcoded ^has_journal and unchecked mkfs/tune2fs
break test
- [Low] ext4/309: unverified su -g/-G support and unchecked space-fill
step
--
commit 6c6eb6186b8e8ccb5609d413ece36c644ad760e0
Author: Sahaj Chaudhari <sahaj123.sc@gmail.com>
ext4: test reserved GID block allocation
This commit adds xfstests test ext4/309 to verify that a user whose primary
and supplementary group matches the superblock s_resgid can allocate blocks
from the reserved pool after ordinary free space is exhausted.
Link: https://lore.kernel.org/r/20261008065651.3517824-1-sahaj123.sc@gmail.com
> diff --git a/tests/ext4/309 b/tests/ext4/309
> new file mode 100755
> index 0000000000000..3bd120091202b
> --- /dev/null
> +++ b/tests/ext4/309
> @@ -0,0 +1,63 @@
[ ... ]
> +MKFS_OPTIONS="-m 10 -N 1024 -O ^has_journal $MKFS_OPTIONS" \
> + _scratch_mkfs_sized $((16 * 1024 * 1024)) 4096 >>"$seqres".full 2>&1
> +"$TUNE2FS_PROG" -u 0 -g "$RESGID" "$SCRATCH_DEV" >>"$seqres".full 2>&1
> +_scratch_mount
[Severity: Medium]
Can formatting with -O ^has_journal here cause _scratch_mount to fail when
journal mount options are configured?
In __ext4_fill_super(), ext4 rejects mounting a journal-less filesystem
whenever any journal mount option (such as data=ordered, data=journal,
data=writeback, or commit=) is passed, which is not filtered out by
_exclude_scratch_mount_option.
Also, appending caller mkfs options after -m 10 -N 1024 -O ^has_journal lets
a preset such as -m 0 override the 10% reserved pool and cause the final
fallocate to fail. Neither _scratch_mkfs_sized nor tune2fs checks its exit
status with _fail, so if mkfs fails on conflicting options (for example
-J size=... with -O ^has_journal), the test mounts the stale filesystem left
on the scratch device and fails when 16M does not fill it.
> +
> +touch "$TEST_FILE"
> +chmod 0666 "$TEST_FILE"
> +
> +su -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -f -c 'pwrite 0 16M' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1
> +
> +if su -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -c 'falloc 16M 4k' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1; then
> + _fail "ordinary user allocated from the reserved pool"
> +fi
> +
> +su -s /bin/bash -g "$RES_GROUP" -G "$RES_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -c 'falloc 16M 4k' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1 || _fail "reserved GID failed to allocate a block"
[Severity: Low]
What happens if su does not support -g and -G, or if $qa_user cannot
traverse the scratch mount point?
The -g and -G flags are specific to util-linux su and are not checked before
use. Because the pwrite step does not verify that any data was written and
the first falloc check treats any non-zero exit status from su as space
exhaustion, an early su or permission failure falls through to the final
step and fails with "reserved GID failed to allocate a block".
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261107193050.330685-1-sahaj123.sc@gmail.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] ext4: test reserved GID block allocation
2026-11-07 19:30 ` [PATCH v2] ext4: test reserved GID block allocation Sahaj Chaudhari
2026-11-07 19:41 ` sashiko-bot
@ 2026-11-09 13:08 ` Jan Kara
2026-11-09 15:54 ` Zorro Lang
2026-11-10 18:52 ` Sahaj Chaudhari
3 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2026-11-09 13:08 UTC (permalink / raw)
To: Sahaj Chaudhari; +Cc: fstests, Zorro Lang, Jan Kara, linux-ext4
On Sun 08-11-26 01:00:50, Sahaj Chaudhari wrote:
> Create a small ext4 filesystem with a nonzero reserved block pool and a
> reserved GID stored in the superblock. Fill ordinary free space as an
> unprivileged user outside that group, verify that another allocation is
> denied, then verify that the same user can allocate a block with the
> reserved group as its primary and only supplementary group.
>
> This exercises loading s_resgid from the superblock rather than overriding
> it with a resgid mount option, and detects the regression fixed by:
>
> commit c87abbab6147 ("ext4: fix incorrect function call when initializing s_resgid")
>
> Use su and xfs_io for the credential changes and allocations instead of
> adding a dedicated C helper. Exclude mount options that override the
> reserved IDs and skip configurations without the required users, groups
> or fallocate support.
>
> Link: https://lore.kernel.org/r/20261008065651.3517824-1-sahaj123.sc@gmail.com
> Signed-off-by: Sahaj Chaudhari <sahaj123.sc@gmail.com>
Thanks! The patch looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> Changes since v1:
> - Replace the C helper and its build registration with su and xfs_io,
> following Jan's and Zorro's suggestions.
> - Set both primary and supplementary groups explicitly, and compare
> ordinary and reserved-GID allocations using the same unprivileged UID.
> - Exclude resuid/resgid mount overrides; leave _user_do unchanged.
>
> Review:
> https://lore.kernel.org/all/j2dthue3fzup753jrndn6kh44qfvihx2bylmiwcoxubnrxpgdk@zx6shyx3hbb6/
> https://lore.kernel.org/all/aurRSqrRkEfyp6M7@zlang-mailbox/
>
> Testing:
> - Exact ./check ext4/309 in x86-64 QEMU: ordinary allocation is denied
> after filling free space; reserved-GID allocation succeeds on the fixed
> kernel. The pre-c87abbab6147 kernel fails at the reserved-GID allocation
> as expected. Both TEST_DEV and SCRATCH_DEV pass e2fsck -fn afterward.
> - Exact ./check ext4/309 passes natively on an AM642 ARM64 board running
> 6.12.57-ti-gd00ea986335d; both loop-backed filesystems pass e2fsck -fn.
> - Validation is limited to ext4/309, not the full xfstests suite.
>
> tests/ext4/309 | 63 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/ext4/309.out | 2 ++
> 2 files changed, 65 insertions(+)
> create mode 100755 tests/ext4/309
> create mode 100644 tests/ext4/309.out
>
> diff --git a/tests/ext4/309 b/tests/ext4/309
> new file mode 100755
> index 00000000..3bd12009
> --- /dev/null
> +++ b/tests/ext4/309
> @@ -0,0 +1,63 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 Sahaj Chaudhari. All Rights Reserved.
> +#
> +# FS QA Test No. 309
> +#
> +# Verify that the ext4 reserved GID can allocate blocks after ordinary
> +# free space is exhausted.
> +#
> +. ./common/preamble
> +_begin_fstest auto enospc quick
> +
> +_exclude_fs ext2
> +_exclude_fs ext3
> +_exclude_scratch_mount_option "resuid=" "resgid="
> +
> +_fixed_by_kernel_commit c87abbab6147 \
> + "ext4: fix incorrect function call when initializing s_resgid"
> +
> +_require_scratch
> +_require_command "$TUNE2FS_PROG" tune2fs
> +_require_xfs_io_command "falloc"
> +_require_user
> +_require_group
> +RES_GROUP=$qa_group
> +_require_group root
> +FILL_GROUP=root
> +
> +echo "Silence is golden"
> +
> +RESGID=$(_cat_group | awk -F: -v group="$RES_GROUP" \
> + '$1 == group { print $3 }')
> +FILLGID=$(_cat_group | awk -F: -v group="$FILL_GROUP" \
> + '$1 == group { print $3 }')
> +[ "$RESGID" != "$FILLGID" ] || _notrun "reserved and fill groups are identical"
> +[ "$(id -u "$qa_user")" -ne 0 ] || _notrun "test user is root"
> +TEST_FILE="$SCRATCH_MNT/ext4_resgid.fill"
> +
> +MKFS_OPTIONS="-m 10 -N 1024 -O ^has_journal $MKFS_OPTIONS" \
> + _scratch_mkfs_sized $((16 * 1024 * 1024)) 4096 >>"$seqres".full 2>&1
> +"$TUNE2FS_PROG" -u 0 -g "$RESGID" "$SCRATCH_DEV" >>"$seqres".full 2>&1
> +_scratch_mount
> +
> +touch "$TEST_FILE"
> +chmod 0666 "$TEST_FILE"
> +
> +su -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -f -c 'pwrite 0 16M' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1
> +
> +if su -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -c 'falloc 16M 4k' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1; then
> + _fail "ordinary user allocated from the reserved pool"
> +fi
> +
> +su -s /bin/bash -g "$RES_GROUP" -G "$RES_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -c 'falloc 16M 4k' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1 || _fail "reserved GID failed to allocate a block"
> +
> +_scratch_unmount
> +status=0
> +exit
> diff --git a/tests/ext4/309.out b/tests/ext4/309.out
> new file mode 100644
> index 00000000..56330d65
> --- /dev/null
> +++ b/tests/ext4/309.out
> @@ -0,0 +1,2 @@
> +QA output created by 309
> +Silence is golden
> --
> 2.43.0
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] ext4: test reserved GID block allocation
2026-11-07 19:30 ` [PATCH v2] ext4: test reserved GID block allocation Sahaj Chaudhari
2026-11-07 19:41 ` sashiko-bot
2026-11-09 13:08 ` Jan Kara
@ 2026-11-09 15:54 ` Zorro Lang
2026-11-10 18:52 ` Sahaj Chaudhari
3 siblings, 0 replies; 5+ messages in thread
From: Zorro Lang @ 2026-11-09 15:54 UTC (permalink / raw)
To: Sahaj Chaudhari; +Cc: fstests, Jan Kara, linux-ext4
On Sun, Nov 08, 2026 at 01:00:50AM +0530, Sahaj Chaudhari wrote:
> Create a small ext4 filesystem with a nonzero reserved block pool and a
> reserved GID stored in the superblock. Fill ordinary free space as an
> unprivileged user outside that group, verify that another allocation is
> denied, then verify that the same user can allocate a block with the
> reserved group as its primary and only supplementary group.
>
> This exercises loading s_resgid from the superblock rather than overriding
> it with a resgid mount option, and detects the regression fixed by:
>
> commit c87abbab6147 ("ext4: fix incorrect function call when initializing s_resgid")
>
> Use su and xfs_io for the credential changes and allocations instead of
> adding a dedicated C helper. Exclude mount options that override the
> reserved IDs and skip configurations without the required users, groups
> or fallocate support.
>
> Link: https://lore.kernel.org/r/20261008065651.3517824-1-sahaj123.sc@gmail.com
> Signed-off-by: Sahaj Chaudhari <sahaj123.sc@gmail.com>
> ---
> Changes since v1:
> - Replace the C helper and its build registration with su and xfs_io,
> following Jan's and Zorro's suggestions.
> - Set both primary and supplementary groups explicitly, and compare
> ordinary and reserved-GID allocations using the same unprivileged UID.
> - Exclude resuid/resgid mount overrides; leave _user_do unchanged.
>
> Review:
> https://lore.kernel.org/all/j2dthue3fzup753jrndn6kh44qfvihx2bylmiwcoxubnrxpgdk@zx6shyx3hbb6/
> https://lore.kernel.org/all/aurRSqrRkEfyp6M7@zlang-mailbox/
>
> Testing:
> - Exact ./check ext4/309 in x86-64 QEMU: ordinary allocation is denied
> after filling free space; reserved-GID allocation succeeds on the fixed
> kernel. The pre-c87abbab6147 kernel fails at the reserved-GID allocation
> as expected. Both TEST_DEV and SCRATCH_DEV pass e2fsck -fn afterward.
> - Exact ./check ext4/309 passes natively on an AM642 ARM64 board running
> 6.12.57-ti-gd00ea986335d; both loop-backed filesystems pass e2fsck -fn.
> - Validation is limited to ext4/309, not the full xfstests suite.
>
> tests/ext4/309 | 63 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/ext4/309.out | 2 ++
> 2 files changed, 65 insertions(+)
> create mode 100755 tests/ext4/309
> create mode 100644 tests/ext4/309.out
>
> diff --git a/tests/ext4/309 b/tests/ext4/309
> new file mode 100755
> index 00000000..3bd12009
> --- /dev/null
> +++ b/tests/ext4/309
> @@ -0,0 +1,63 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 Sahaj Chaudhari. All Rights Reserved.
> +#
> +# FS QA Test No. 309
> +#
> +# Verify that the ext4 reserved GID can allocate blocks after ordinary
> +# free space is exhausted.
> +#
> +. ./common/preamble
> +_begin_fstest auto enospc quick
> +
> +_exclude_fs ext2
> +_exclude_fs ext3
> +_exclude_scratch_mount_option "resuid=" "resgid="
> +
> +_fixed_by_kernel_commit c87abbab6147 \
> + "ext4: fix incorrect function call when initializing s_resgid"
> +
> +_require_scratch
> +_require_command "$TUNE2FS_PROG" tune2fs
> +_require_xfs_io_command "falloc"
> +_require_user
> +_require_group
> +RES_GROUP=$qa_group
> +_require_group root
> +FILL_GROUP=root
Is "root" necessary? xfstests doesn't typically use root group this way.
If you need two test users or groups, you can use "fsgqa2" or other
accounts in addition to the default "fsgqa". Refer to generic/698 for
an example.
> +
> +echo "Silence is golden"
> +
> +RESGID=$(_cat_group | awk -F: -v group="$RES_GROUP" \
> + '$1 == group { print $3 }')
> +FILLGID=$(_cat_group | awk -F: -v group="$FILL_GROUP" \
> + '$1 == group { print $3 }')
> +[ "$RESGID" != "$FILLGID" ] || _notrun "reserved and fill groups are identical"
> +[ "$(id -u "$qa_user")" -ne 0 ] || _notrun "test user is root"
> +TEST_FILE="$SCRATCH_MNT/ext4_resgid.fill"
> +
> +MKFS_OPTIONS="-m 10 -N 1024 -O ^has_journal $MKFS_OPTIONS" \
This test already imposes fairly strict mkfs options on its own, so I don't
think it's necessary to inherit external MKFS_OPTIONS. It adds little value
and can easily cause mkfs to fail and abort the test I think.
> + _scratch_mkfs_sized $((16 * 1024 * 1024)) 4096 >>"$seqres".full 2>&1
> +"$TUNE2FS_PROG" -u 0 -g "$RESGID" "$SCRATCH_DEV" >>"$seqres".full 2>&1
> +_scratch_mount
> +
> +touch "$TEST_FILE"
> +chmod 0666 "$TEST_FILE"
> +
> +su -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -f -c 'pwrite 0 16M' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1
> +
> +if su -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -c 'falloc 16M 4k' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1; then
> + _fail "ordinary user allocated from the reserved pool"
> +fi
> +
> +su -s /bin/bash -g "$RES_GROUP" -G "$RES_GROUP" "$qa_user" -c \
> + "$XFS_IO_PROG -c 'falloc 16M 4k' '$TEST_FILE'" \
> + >>"$seqres".full 2>&1 || _fail "reserved GID failed to allocate a block"
> +
> +_scratch_unmount
If this's a part of test steps, we can remove this line.
> +status=0
> +exit
_exit 0
Thanks,
Zorro
> diff --git a/tests/ext4/309.out b/tests/ext4/309.out
> new file mode 100644
> index 00000000..56330d65
> --- /dev/null
> +++ b/tests/ext4/309.out
> @@ -0,0 +1,2 @@
> +QA output created by 309
> +Silence is golden
> --
> 2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] ext4: test reserved GID block allocation
2026-11-07 19:30 ` [PATCH v2] ext4: test reserved GID block allocation Sahaj Chaudhari
` (2 preceding siblings ...)
2026-11-09 15:54 ` Zorro Lang
@ 2026-11-10 18:52 ` Sahaj Chaudhari
3 siblings, 0 replies; 5+ messages in thread
From: Sahaj Chaudhari @ 2026-11-10 18:52 UTC (permalink / raw)
To: fstests; +Cc: Zorro Lang, Jan Kara, linux-ext4
Hi,
I've posted v3 in a new thread, addressing Zorro's suggestions and the
setup/error-checking concerns:
https://lore.kernel.org/fstests/20261110182040.798390-1-sahaj123.sc@gmail.com/
It supersedes v2:
https://lore.kernel.org/fstests/20261107193050.330685-1-sahaj123.sc@gmail.com/
Sorry for posting v3 separately without linking back to this thread.
Thanks,
Sahaj
^ permalink raw reply [flat|nested] 5+ messages in thread