Re: [PATCH 2/2] selftests/seccomp: Ensure that notifications come in FIFO order

[Tycho Andersen <tycho@tycho.pizza>]

On Thu, Apr 28, 2022 at 09:38:10AM -0700, Sargun Dhillon wrote:
> On Thu, Apr 28, 2022 at 6:15 AM Tycho Andersen <tycho@tycho.pizza> wrote:
> > > +     for (i = 0; i < ARRAY_SIZE(pids); i++) {
> > > +             pid = fork();
> > > +             if (pid == 0) {
> > > +                     ret = syscall(__NR_getppid);
> > > +                     exit(ret != USER_NOTIF_MAGIC);
> > > +             }
> > > +             pids[i] = pid;
> > > +     }
> > > +
> > > +     /* This spins until all of the children are sleeping */
> > > +restart_wait:
> > > +     for (i = 0; i < ARRAY_SIZE(pids); i++) {
> > > +             if (get_proc_stat(pids[i]) != 'S') {
> > > +                     nanosleep(&delay, NULL);
> > > +                     goto restart_wait;
> > > +             }
> > > +     }
> >
> > I wonder if we should/can combine this loop with the previous one, and
> > wait for the child to sleep in getppid() before we fork the next one.
> > Otherwise isn't racy in the case that your loop continues to the next
> > iteration before the child processes are scheduled, so things might be
> > out of order? Maybe I'm missing something.
> >
> > In any case, this change seems reasonable to me.
> >
> > Tycho
> It's okay if the child processes are started out of order. The test just
> verifies that the calls are delivered in FIFO order according to when
> the syscall was called (not when the process was started), and we do
> this by just looking at the notification ID. It doesn't care about which
> process generated the notification.

I totally missed that you don't this, I just assumed you did. Thanks.

Anyway, you can add:

Acked-by: Tycho Andersen <tycho@tycho.pizza>

to both patches.

Tycho