Re: [PATCH v3] ext4: test reserved GID block allocation

[Zorro Lang <zlang@kernel.org>]

On Tue, Nov 10, 2026 at 11:50:40PM +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")
> 
> Link: https://lore.kernel.org/r/20261008065651.3517824-1-sahaj123.sc@gmail.com
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Sahaj Chaudhari <sahaj123.sc@gmail.com>
> ---
> Changes since v2:
> - Use fsgqa2 instead of root for the fill group, and require it explicitly.
> - Do not inherit external MKFS_OPTIONS; exclude journal-dependent mount
>   options for the test's no-journal filesystem.
> - Check tune2fs and su -g/-G support, and use the validated su executable
>   consistently.
> - Require the fill operation to write data and reach ENOSPC; check that
>   ordinary allocation also fails specifically with ENOSPC.
> - Let the check harness unmount scratch and finish with _exit 0.
> - Add Jan Kara's Reviewed-by.

Thanks, this version looks good to me now.

Reviewed-by: Zorro Lang <zlang@kernel.org>

> 
> Testing:
> - ./check ext4/309 in isolated x86-64 QEMU with fresh TEST_DEV and
>   SCRATCH_DEV images passes on the fixed kernel and fails specifically
>   at reserved-GID allocation on the pre-c87abbab6147 kernel.
> - Both filesystems pass e2fsck -fn after each run.
> - bash -n and git diff --check pass. Validation is limited to ext4/309,
>   not the full xfstests suite.
> 
>  tests/ext4/309     | 75 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/ext4/309.out |  2 ++
>  2 files changed, 77 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..e5dcd215
> --- /dev/null
> +++ b/tests/ext4/309
> @@ -0,0 +1,75 @@
> +#! /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=" "data=" \
> +	"data_err=abort" "commit=" "journal_async_commit" "journal_checksum"
> +
> +_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 fsgqa2
> +FILL_GROUP=fsgqa2
> +SU_PROG=$(command -v su) || _notrun "su utility required"
> +"$SU_PROG" -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" "$qa_user" \
> +	-c true >/dev/null 2>&1 ||
> +	_notrun "su with -g and -G support required"
> +
> +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" \
> +	_scratch_mkfs_sized $((16 * 1024 * 1024)) 4096 >>"$seqres".full 2>&1
> +"$TUNE2FS_PROG" -u 0 -g "$RESGID" "$SCRATCH_DEV" >>"$seqres".full 2>&1 ||
> +	_fail "could not set scratch filesystem reserved GID"
> +_scratch_mount
> +
> +touch "$TEST_FILE"
> +chmod 0666 "$TEST_FILE"
> +
> +FILL_OUTPUT=$("$SU_PROG" -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" \
> +	"$qa_user" -c "$XFS_IO_PROG -f -c 'pwrite 0 16M' '$TEST_FILE'" 2>&1)
> +FILL_STATUS=$?
> +printf '%s\n' "$FILL_OUTPUT" >>"$seqres".full
> +printf '%s\n' "$FILL_OUTPUT" | grep -q "pwrite: No space left on device" ||
> +	_fail "space-fill operation did not reach ENOSPC (status $FILL_STATUS)"
> +FILL_SIZE=$(_get_filesize "$TEST_FILE") ||
> +	_fail "could not determine space-fill file size"
> +[ "$FILL_SIZE" -gt 0 ] || _fail "space-fill operation wrote no data"
> +
> +if ALLOC_OUTPUT=$("$SU_PROG" -s /bin/bash -g "$FILL_GROUP" -G "$FILL_GROUP" \
> +	"$qa_user" -c "$XFS_IO_PROG -c 'falloc 16M 4k' '$TEST_FILE'" 2>&1); then
> +	_fail "ordinary user allocated from the reserved pool"
> +fi
> +printf '%s\n' "$ALLOC_OUTPUT" >>"$seqres".full
> +printf '%s\n' "$ALLOC_OUTPUT" | grep -q "fallocate: No space left on device" ||
> +	_fail "ordinary allocation failed for a reason other than ENOSPC"
> +
> +"$SU_PROG" -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"
> +
> +_exit 0
> 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
> 
> base-commit: d07dca72d2455f4ba8c660ad4c008541ccccb707
> -- 
> 2.43.0
>