Download raw body.
> From: "Theo de Raadt" <deraadt@openbsd.org>
> Date: Sun, 23 Feb 2025 09:21:26 -0700
>
> > 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.
Static binaries can't get there because they will have
VM_MAP_PINSYSCALL_ONCE set. Do you mean a specially crafted ld.so?
> 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.
Is there any reason why we can't set VM_MAP_PINSYSCALL_ONCE early in
the syscall? Then we can also simplify the EPERM condition. Something like:
if (pr->ps_vmspace->vm_map.flags & VM_MAP_PINSYSCALL_ONCE)
return (EPERM);
mtx_enter(&pr->ps_vmspace->vm_map.flags_lock);
pr->ps_vmspace->vm_map.flags |= VM_MAP_PINSYSCALL_ONCE;
mtx_leave(&pr->ps_vmspace->vm_map.flags_lock);
...
This doesn mean that if someone invokes pinsyscalls(2) with bogus
parameters they will have blown their one chance at it. But I'd argue
that's a good thing ;).
> Index: uvm_mmap.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_mmap.c,v
> diff -u -p -u -r1.193 uvm_mmap.c
> --- uvm_mmap.c 14 Dec 2024 12:07:38 -0000 1.193
> +++ uvm_mmap.c 23 Feb 2025 16:20:01 -0000
> @@ -648,6 +648,10 @@ err:
> free(pins, M_PINSYSCALL, npins * sizeof(u_int));
> return (error);
> }
> + if (pr->ps_libcpin.pn_pins) { /* raced by another static thread */
> + error = EPERM;
> + goto err;
> + }
> pr->ps_libcpin.pn_start = base;
> pr->ps_libcpin.pn_end = base + len;
> pr->ps_libcpin.pn_pins = pins;
>
>