[LTP] [PATCH] syscalls/fanotify: Kill the child process before exit

[Amir Goldstein]

On Fri, Nov 5, 2021 at 5:01 AM Matthew Bobrowski <repnop@google.com> wrote:
>
> On Fri, Nov 05, 2021 at 02:22:51AM +0000, zhaogongyi wrote:
> > Hi,
> >
> > >
> > > On Thu, Nov 04, 2021 at 07:37:35PM +0100, Petr Vorel wrote:
> > > > Hi all,
> > > >
> > > > > Before the main process exit abnormally, we need to kill the child
> > > > > process.
> > > >
> > > > Amir, Matthew, could you please have a look?
> > >
> > > If anything, I feel as though stop_children() should probably be called from
> > > cleanup() as that callback will be invoked if any of the
> > > SAFE_FANOTIFY_* macros fail anyway, right?
> >
> > It seems that calling before run_children, we need not cleanup though the SAFE_FANOTIFY_* macros fail.
>
> Even if stop_children() is called prior to run_children(), maybe it would
> be OK given the fact that child_pid[] is a global and pre-intialized with
> zeros. Meaning, that if SAFE_KILL() was called passing a PID value of 0,
> then all processes part of the calling process' process group would be sent
> a SIGKILL signal, which is kind of what we want anyway when taking a
> teardown path. This is just a quick thought.
>

This hack is not necessary.

> Nonetheless, you could also introduce barriers into stop_children()
> i.e. checking whether PID values in child_pid[] are all zero and if so
> return early. That'd prevent you from taking any unnecessary cleanup path
> in the event that something fails before any child processes are spawned?
>

This would be the obvious solution.
But simpler to skip zero values then stop the iterator.
This makes the cleanup helper reenetrant:

static int stop_children(void)
{
        int child_ret;
        int i, ret = 0;

        for (i = 0; i < MAX_CHILDREN; i++) {
                if (!child_pid[i]) continue;
                SAFE_KILL(child_pid[i], SIGKILL);
        }
        for (i = 0; i < MAX_CHILDREN; i++) {
                if (!child_pid[i]) continue;
                SAFE_WAITPID(child_pid[i], &child_ret, 0);
                if (!WIFSIGNALED(child_ret))
                        ret = 1;
                child_pid[i] = 0;
       }

        return ret;
}

Thanks,
Amir.