[LTP] [PATCH v1] Fix: dirtyc0w_shmem child process exit with error due to uninitialized lib_pid

[Li Wang]

On Sat, May 10, 2025 at 4:54 PM Wei Gao via ltp <ltp@lists.linux.it> wrote:
>
> The dirtyc0w_shmem test spawns a child process using execlp. This child process then calls tst_brk(), which exits early
> with a non-zero status because execlp does not inherit the parent's lib_pid variable. Consequently, the parent process
> incorrectly reports an "Invalid child exit value".
>
> This commit addresses this by ensuring the child process has access to the necessary lib_pid and main_pid by passing
> them through a shared results structure. This prevents the premature exit in the child and the subsequent error report
> in the parent.
>
> Related commit:
> commit a1f82704c28d9e027ca899f5ca2841e7fe49de72
> lib/tst_test.c: Fix tst_brk() handling
>
> Detail failure log:
> tst_tmpdir.c:317: TINFO: Using /tmp/LTP_dirSOGVBC as tmpdir (btrfs filesystem)
> tst_test.c:1938: TINFO: LTP version: 20250507.4a0e3a8fa
> tst_test.c:1942: TINFO: Tested kernel: 6.4.0-150700.51-default #1 SMP Wed Apr 30 21:35:43 UTC 2025 (6930611) s390x
> tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:678: TINFO: CONFIG_FAULT_INJECTION kernel option detected which might slow the execution
> tst_test.c:1760: TINFO: Overall timeout per run is 0h 04m 00s
> dirtyc0w_shmem.c:54: TINFO: Mounting tmp_dirtyc0w_shmem to /tmp/LTP_dirSOGVBC/tmp_dirtyc0w_shmem fstyp=tmpfs flags=0
> dirtyc0w_shmem_child.c:160: TCONF: System does not have userfaultfd minor fault support for shmem <<<<<<<<<< 1
> tst_test.c:481: TBROK: Invalid child (8163) exit value 32 <<<<<<<< 2
> dirtyc0w_shmem.c:102: TINFO: Umounting /tmp/LTP_dirSOGVBC/tmp_dirtyc0w_shmem
>
> tmp_dirtyc0w_shmem.c call execlp to create new process run dirtyc0w_shmem_child bin.
>
> SAFE_EXECLP("dirtyc0w_shmem_child", "dirtyc0w_shmem_child", NULL)
>
> Within dirtyc0w_shmem_child.c trigger
>
> tst_brk(TCONF, "System does not have userfaultfd minor fault support for shmem")
>
> Since execlp does not inherit the parent process's variables lib_pid, so it will return TCONF(32) directly.
>
> void tst_vbrk_(const char *file, const int lineno, int ttype, const char *fmt,
>                va_list va)
> {
> ...
>         if (!lib_pid)
>                 exit(TTYPE_RESULT(ttype));   <<<<<
> ...
> }
>
> So finally captured by check_child_status report an error.
>
> static void check_child_status(pid_t pid, int status)
> {
> ...
>         if (WEXITSTATUS(status))
>                 tst_brk(TBROK, "Invalid child (%i) exit value %i", pid, WEXITSTATUS(status));  <<<<
> }
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  lib/tst_test.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 2bb4519dd..b666715b9 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -59,7 +59,7 @@ static const char *tid;
>  static int iterations = 1;
>  static float duration = -1;
>  static float timeout_mul = -1;
> -static pid_t main_pid, lib_pid;
> +static pid_t lib_pid;
>  static int mntpoint_mounted;
>  static int ovl_mounted;
>  static struct timespec tst_start_time; /* valid only for test pid */
> @@ -78,6 +78,8 @@ struct results {
>         int abort_flag;
>         unsigned int runtime;
>         unsigned int overall_time;
> +       pid_t lib_pid;
> +       pid_t main_pid;
>  };

Can we avoid polluting the struct results with main_pid and lib_pid?
>From a design separation standpoint, results should only store test
outcome data, not test infrastructure state.

As LTP library already provides a tst_reinit() function for child processes
spawned via exec()/execlp() to restore their test context.

We could consider two approaches:

1. Create a separate IPC region to store infrastructure state(e.g.,
main_pid, lib_pid)
in a new struct tst_meta_info. The child can then access this data via
tst_reinit()
without modifying the struct results.

2. Simply pass main_pid and lib_pid through environment variables in the
ltp library, and retrieve them from tst_reinit() in the child.

Or, maybe others have simpler options. Cc'ing them.

In either case, we should set 'tst_test->child_need_reinit' in the parent.

@Cyril, @Petr, I support merging this fix before the May release, as I’ve also
encountered the same failure during my pre-release testing.


-- 
Regards,
Li Wang