Mateusz Guzik <mjguzik@gmail.com> wrote:

> On Sun, Feb 23, 2025 at 5:21 PM Theo de Raadt <deraadt@openbsd.org> wrote:
> >
> > > Then this copyin:
> > >           error = copyin(SCARG(uap, pins), pins, npins * sizeof(u_int));
> > >           if (error)
> > >                   goto err;
> > >
> > > will go off cpu and drop the kernel lock, allowing other threads to
> > > reach the same point, all still observing pr->ps_libcpin.pn_start == 0.
> > >
> > > Then several threads will reach this assignment:
> > >           pr->ps_libcpin.pn_start = base;
> > >
> > > is going to override all previously set bufs, leaking them.
> >
> > Yes, would need very custom crt0 in those static binaries.
> >
> 
> Presumably someone wanting to mess with the kernel would go there.
> 
> > This diff should allow the first thread to do pinsyscalls to win, and
> > re-check the condition post-sleep so that others lose and free their
> > pintable.  EPERM matches the existing spec in the man page.
> >
> 
> Another angle is what happens when one thread is messing about with
> installing the table, while another is issuing syscalls. Note there is
> no synchronization between this and pin_check. It may be this does not
> blow up in practice at the moment, but I don't think depending on it
> is viable.

That is not a concern.  system calls which start before pinsyscalls
completes setup, work as expected.

> The safest and easiest way out that I see is to only allow pinsyscalls
> if there is just one thread, assuming multithreaded usage was not
> meant to happen to begin with.

No, that has to be wrong.  All a program needs to do for system call
locking not to occur, is to tfork first???