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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YmrsQZ2lNGHjGK6i@cisco>
Date:   Thu, 28 Apr 2022 13:34:25 -0600
From:   Tycho Andersen <tycho@...ho.pizza>
To:     Sargun Dhillon <sargun@...gun.me>
Cc:     Kees Cook <keescook@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Christian Brauner <christian.brauner@...ntu.com>,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 2/2] selftests/seccomp: Ensure that notifications come in
 FIFO order

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@...ho.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@...ho.pizza>

to both patches.

Tycho