On Tue, May 17, 2022 at 10:14:18AM -0600, Theo de Raadt wrote: > Martin Pieuchot <m...@openbsd.org> wrote: > > > On 17/05/22(Tue) 10:44, David Gwynne wrote: > > > this narrows the scope of the KERNEL_LOCK in kbind(2) so the syscall > > > argument checks can be done without the kernel lock. > > > > > > care is taken to allow the pc/cookie checks to run without any lock by > > > being careful with the order of the checks. all modifications to the > > > pc/cookie state are serialised by the per process mutex. > > > > I don't understand why it is safe to do the following check without > > holding a mutex:
because you can't read any invalid intermediate value. in the fast path here, either it is the expected value when you're using the interface correctly, or it isnt. if it isn't then you take the "slow" path by taking the mutex and doing the comprehensive value checks and updates. > > > > if (pr->ps_kbind_addr == pc) > > ... > > > > Is there much differences when always grabbing the per-process mutex? sure, that sequence is maybe half a dozen instructions without the mutex, and many times that with the mutex. from a different perspective, i can't see a big difference between kernel compile times with or without this diff, so right now the mutex isn't going to make a difference either. from yet another perspective, broad use of a common lock can end up hurting in the long run because you may end up where everything is serialised and you have to go back and do a ton of work again anyway. > I think the theory is ps_kbind_addr is fixed to a shared address space > in "pr", if you are threaded there is only one ps_kbind_addr for all the > processes "p" sharing that address space. yes. > And execve() uses single_thread_set, which means you can't race? also yes. the value starts at 0, updates are serialised by the mutex, and the updates don't produce invalid intermediate state.
