On Sun, Feb 23, 2025 at 12:50:12PM -0700, Theo de Raadt wrote:
> 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???
> 

So what about the |MZERO
which was my question in the first place ?


If I'm correct, it looks like it's 100% not needed since the copy is
going to overwrite the full area, and there's no data left there
(I thought M_PINSYSCALL might be a separate area).

I understand if it's a leftover from a previous draft.