Re: start unlocking kbind(2)

2022-06-24 Thread Scott Cheloha
On Wed, Jun 15, 2022 at 10:40:35PM -0500, Scott Cheloha wrote: > On Wed, Jun 15, 2022 at 06:17:07PM -0600, Theo de Raadt wrote: > > Mark Kettenis wrote: > > > > > Well, I believe that Scott was trying to fix a race condition that can > > > only happen if code is using kbind(2) incorrectly, i.e.

Re: start unlocking kbind(2)

2022-06-15 Thread Scott Cheloha
On Wed, Jun 15, 2022 at 06:17:07PM -0600, Theo de Raadt wrote: > Mark Kettenis wrote: > > > Well, I believe that Scott was trying to fix a race condition that can > > only happen if code is using kbind(2) incorrectly, i.e. when the > > threads deliberately pass different cookies to kbind(2) or

Re: start unlocking kbind(2)

2022-06-15 Thread Theo de Raadt
Mark Kettenis wrote: > Well, I believe that Scott was trying to fix a race condition that can > only happen if code is using kbind(2) incorrectly, i.e. when the > threads deliberately pass different cookies to kbind(2) or execute > kbind(2) from different "text" addresses. > > I still think the

Re: start unlocking kbind(2)

2022-06-15 Thread Mark Kettenis
> From: Philip Guenther > Date: Wed, 15 Jun 2022 10:07:06 -0900 > > On Mon, 13 Jun 2022, Theo de Raadt wrote: > > Scott Cheloha wrote: > > > > Am I wrong that kbind is never called twice in the same address space? > > > > > > Isn't this exactly what happened the last time we tried this? > > >

Re: start unlocking kbind(2)

2022-06-15 Thread Philip Guenther
On Mon, 13 Jun 2022, Theo de Raadt wrote: > Scott Cheloha wrote: > > > Am I wrong that kbind is never called twice in the same address space? > > > > Isn't this exactly what happened the last time we tried this? > > Tried what? kbind has never been NOLOCK. Scott's referring to rev 1.237 of

Re: start unlocking kbind(2)

2022-06-13 Thread Theo de Raadt
Scott Cheloha wrote: > > Am I wrong that kbind is never called twice in the same address space? > > Isn't this exactly what happened the last time we tried this? Tried what? kbind has never been NOLOCK. > > Can someone describe a synthetic sequence where racing sys_kbind calls > > perform

Re: start unlocking kbind(2)

2022-06-13 Thread Scott Cheloha
On Sun, Jun 12, 2022 at 12:12:33AM -0600, Theo de Raadt wrote: > David Gwynne wrote: > > > On Wed, May 18, 2022 at 07:42:32PM -0600, Theo de Raadt wrote: > > > Mark Kettenis wrote: > > > > > > > > Isn't the vm_map_lock enough? > > > > > > > > Could be. The fast path is going to take that

Re: start unlocking kbind(2)

2022-06-12 Thread Theo de Raadt
David Gwynne wrote: > On Wed, May 18, 2022 at 07:42:32PM -0600, Theo de Raadt wrote: > > Mark Kettenis wrote: > > > > > > Isn't the vm_map_lock enough? > > > > > > Could be. The fast path is going to take that lock anyway. This > > > would require a bit of surgery to uvm_map_extract() to

Re: start unlocking kbind(2)

2022-06-11 Thread David Gwynne
On Tue, May 31, 2022 at 04:48:16PM +0200, Martin Pieuchot wrote: > On 18/05/22(Wed) 15:53, Alexander Bluhm wrote: > > On Tue, May 17, 2022 at 10:44:54AM +1000, David Gwynne wrote: > > > + cookie = SCARG(uap, proc_cookie); > > > + if (pr->ps_kbind_addr == pc) { > > > +

Re: start unlocking kbind(2)

2022-06-11 Thread David Gwynne
On Wed, May 18, 2022 at 07:42:32PM -0600, Theo de Raadt wrote: > Mark Kettenis wrote: > > > > Isn't the vm_map_lock enough? > > > > Could be. The fast path is going to take that lock anyway. This > > would require a bit of surgery to uvm_map_extract() to make sure we > > don't take the

Re: start unlocking kbind(2)

2022-05-31 Thread Martin Pieuchot
On 18/05/22(Wed) 15:53, Alexander Bluhm wrote: > On Tue, May 17, 2022 at 10:44:54AM +1000, David Gwynne wrote: > > + cookie = SCARG(uap, proc_cookie); > > + if (pr->ps_kbind_addr == pc) { > > + membar_datadep_consumer(); > > + if (pr->ps_kbind_cookie != cookie) > > +

Re: start unlocking kbind(2)

2022-05-18 Thread Theo de Raadt
Mark Kettenis wrote: > > Isn't the vm_map_lock enough? > > Could be. The fast path is going to take that lock anyway. This > would require a bit of surgery to uvm_map_extract() to make sure we > don't take the vm_map_lock twice. Worth exploring I'd say. I think the vm_map_lock can be

Re: start unlocking kbind(2)

2022-05-18 Thread Mark Kettenis
> From: "Theo de Raadt" > Date: Wed, 18 May 2022 10:48:01 -0600 > > Mark Kettenis wrote: > > > > I guess there is another question -- should the ps_kbind_* variables > > > be stored inside the uvmspace, rather than inside pr? > > > > I think there are arguments for both. But I don't think

Re: start unlocking kbind(2)

2022-05-18 Thread Theo de Raadt
Mark Kettenis wrote: > > I guess there is another question -- should the ps_kbind_* variables > > be stored inside the uvmspace, rather than inside pr? > > I think there are arguments for both. But I don't think moving it > makes things easier. Whatever proc sets the kbind configuration,

Re: start unlocking kbind(2)

2022-05-18 Thread Mark Kettenis
> From: "Theo de Raadt" > Date: Wed, 18 May 2022 10:09:38 -0600 > > I guess there is another question -- should the ps_kbind_* variables > be stored inside the uvmspace, rather than inside pr? I think there are arguments for both. But I don't think moving it makes things easier.

Re: start unlocking kbind(2)

2022-05-18 Thread Mark Kettenis
> From: "Theo de Raadt" > Date: Wed, 18 May 2022 10:05:03 -0600 > > David Gwynne wrote: > > > 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

Re: start unlocking kbind(2)

2022-05-18 Thread Theo de Raadt
I guess there is another question -- should the ps_kbind_* variables be stored inside the uvmspace, rather than inside pr?

Re: start unlocking kbind(2)

2022-05-18 Thread Theo de Raadt
David Gwynne wrote: > 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

Re: start unlocking kbind(2)

2022-05-18 Thread Alexander Bluhm
On Tue, May 17, 2022 at 10:44:54AM +1000, David Gwynne wrote: > + cookie = SCARG(uap, proc_cookie); > + if (pr->ps_kbind_addr == pc) { > + membar_datadep_consumer(); > + if (pr->ps_kbind_cookie != cookie) > + goto sigill; > + } else { You

Re: start unlocking kbind(2)

2022-05-18 Thread David Gwynne
On Tue, May 17, 2022 at 10:14:18AM -0600, Theo de Raadt wrote: > Martin Pieuchot 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

Re: start unlocking kbind(2)

2022-05-17 Thread Theo de Raadt
Martin Pieuchot 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

Re: start unlocking kbind(2)

2022-05-17 Thread Martin Pieuchot
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