linux-ext4 - Re: [PATCH v2 4/5] common/config: Introduce

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87o6xcv7pp.fsf@gmail.com>
Date: Fri, 04 Apr 2025 10:33:46 +0530
From: Ritesh Harjani (IBM) <ritesh.list@...il.com>
To: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>, fstests@...r.kernel.org
Cc: linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org, ojaswin@...ux.ibm.com, djwong@...nel.org, zlang@...nel.org, david@...morbit.com, nirjhar.roy.lists@...il.com
Subject: Re: [PATCH v2 4/5] common/config: Introduce _exit wrapper around exit command

"Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com> writes:

> We should always set the value of status correctly when we are exiting.
> Else, "$?" might not give us the correct value.
> If we see the following trap
> handler registration in the check script:
>
> if $OPTIONS_HAVE_SECTIONS; then
>      trap "_kill_seq; _summary; exit \$status" 0 1 2 3 15
> else
>      trap "_kill_seq; _wrapup; exit \$status" 0 1 2 3 15
> fi
>
> So, "exit 1" will exit the check script without setting the correct
> return value. I ran with the following local.config file:
>
> [xfs_4k_valid]
> FSTYP=xfs
> TEST_DEV=/dev/loop0
> TEST_DIR=/mnt1/test
> SCRATCH_DEV=/dev/loop1
> SCRATCH_MNT=/mnt1/scratch
>
> [xfs_4k_invalid]
> FSTYP=xfs
> TEST_DEV=/dev/loop0
> TEST_DIR=/mnt1/invalid_dir
> SCRATCH_DEV=/dev/loop1
> SCRATCH_MNT=/mnt1/scratch
>
> This caused the init_rc() to catch the case of invalid _test_mount
> options. Although the check script correctly failed during the execution
> of the "xfs_4k_invalid" section, the return value was 0, i.e "echo $?"
> returned 0. This is because init_rc exits with "exit 1" without
> correctly setting the value of "status". IMO, the correct behavior
> should have been that "$?" should have been non-zero.

Nice catch. Feel free to add:

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@...il.com>


>
> The next patch will replace exit with _exit.
>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@...il.com>
> ---
>  common/config | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/common/config b/common/config
> index 79bec87f..eb6af35a 100644
> --- a/common/config
> +++ b/common/config
> @@ -96,6 +96,14 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
>  
>  export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
>  
> +# This functions sets the exit code to status and then exits. Don't use
> +# exit directly, as it might not set the value of "status" correctly.
> +_exit()
> +{
> +	status="$1"
> +	exit "$status"
> +}
> +
>  # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
>  set_mkfs_prog_path_with_opts()
>  {
> -- 
> 2.34.1