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

[Petr Vorel <pvorel@suse.cz>]

> 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;

Wei, you probably noticed tests fail (thanks to Andrea's CI improvements).
It broke at least tst_needs_cmds02.c I guess due missing main_pid:

$ gdb ./tst_needs_cmds02
...
(gdb) run
...
tst_cmd.c:268: TCONF: Couldn't find 'mkfs.ext45' in $PATH

Program received signal SIGSEGV, Segmentation fault.
0x000055555555bea4 in tst_vbrk_ (file=<optimized out>, lineno=<optimized out>, ttype=<optimized out>, fmt=<optimized out>, va=<optimized out>) at tst_test.c:397
397		if (!results->lib_pid)
(gdb) bt
#0  0x000055555555bea4 in tst_vbrk_ (file=<optimized out>, lineno=<optimized out>, ttype=<optimized out>, fmt=<optimized out>, va=<optimized out>) at tst_test.c:397
#1  0x000055555555b5be in tst_brk_ (file=file@entry=0x55555557de1c "tst_cmd.c", lineno=lineno@entry=268, ttype=ttype@entry=32, fmt=fmt@entry=0x55555557deed "Couldn't find '%s' in $PATH")
    at tst_test.c:460
#2  0x000055555556bc57 in tst_check_cmd (cmd=0x55555557c643 "mkfs.ext45 >= 1.43.0", brk_nosupp=brk_nosupp@entry=1) at tst_cmd.c:268
#3  0x000055555555e641 in do_setup (argc=1, argv=0x7fffffffcaa8) at tst_test.c:1363
#4  tst_run_tcases (argc=1, argv=0x7fffffffcaa8, self=self@entry=0x55555558a720 <test>) at tst_test.c:1935
#5  0x000055555555b044 in main (argc=<optimized out>, argv=<optimized out>) at ../../include/tst_test.h:729

> >  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.

+1

> 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.

I suppose using env variables would be simpler but IPC more robust. Better
question to Cyril and Jan.

> 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.

+1

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp