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

[Cyril Hrubis]

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

We do have the abort_flag and runtime there and checkpoints use the
memory after this structure as well, so I wouldn't be so strict.

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

I would like to keep a single IPC region because we have to pass a
path to it to all children too.

Also given that the minimal amount of memory we can allocate for IPC is
a page we can as well have two structures, one for data and one for
library infrastructure bits and place these structures in there
manually. All we need to do is to make sure that we place them at
aligned offsets.

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

That would work too, but this adds more complexity.

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

Yes, please, it's good enough even if it's going to be a temporary
solution.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz