From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <518BF8FF.1000003@web.de> Date: Thu, 09 May 2013 21:29:03 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <5144D3E9.2050205@xenomai.org> <5144D524.4000302@web.de> <5144D5AB.3080004@xenomai.org> <5144D5E3.9050601@web.de> <5144DB0F.9050901@xenomai.org> <518BDD9D.10804@xenomai.org> <518BE3C5.4090203@web.de> <518BE49D.2010404@xenomai.org> <518BE4CE.6020107@web.de> <518BE4FC.6060507@xenomai.org> <518BE6D2.4090304@web.de> <518BEAA2.1060703@xenomai.org> <518BEC1B.7080000@web.de> <518BECA5.3020602@xenomai.org> <518BEDEC.7060302@web.de> <518BF5F7.5070205@xenomai.org> <518BF710.9030500@xenomai.org> In-Reply-To: <518BF710.9030500@xenomai.org> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: Re: [Xenomai] __get_user/__put_user List-Id: Discussions about the Xenomai project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Philippe Gerum Cc: Xenomai On 2013-05-09 21:20, Philippe Gerum wrote: > On 05/09/2013 09:16 PM, Gilles Chanteperdrix wrote: >> On 05/09/2013 08:41 PM, Jan Kiszka wrote: >> >>> On 2013-05-09 20:36, Gilles Chanteperdrix wrote: >>>> On 05/09/2013 08:34 PM, Jan Kiszka wrote: >>>> >>>>> On 2013-05-09 20:27, Gilles Chanteperdrix wrote: >>>>>> On 05/09/2013 08:11 PM, Jan Kiszka wrote: >>>>>> >>>>>>> On 2013-05-09 20:03, Gilles Chanteperdrix wrote: >>>>>>>> On 05/09/2013 08:02 PM, Jan Kiszka wrote: >>>>>>>> >>>>>>>>> On 2013-05-09 20:02, Gilles Chanteperdrix wrote: >>>>>>>>>> On 05/09/2013 07:58 PM, Jan Kiszka wrote: >>>>>>>>>> >>>>>>>>>>> On 2013-05-09 19:32, Gilles Chanteperdrix wrote: >>>>>>>>>>>> On 03/16/2013 09:50 PM, Gilles Chanteperdrix wrote: >>>>>>>>>>>> >>>>>>>>>>>>> On 03/16/2013 09:28 PM, Jan Kiszka wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> On 2013-03-16 21:27, Gilles Chanteperdrix wrote: >>>>>>>>>>>>>>> On 03/16/2013 09:25 PM, Jan Kiszka wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On 2013-03-16 21:19, Gilles Chanteperdrix wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> xenomai asm-generic/syscall.h defines >>>>>>>>>>>>>>>>> __xn_put_user/__xn_get_user as >>>>>>>>>>>>>>>>> aliases for __put_user/__get_user, which >>>>>>>>>>>>>>>>> implementation, for the ARM >>>>>>>>>>>>>>>>> architecture calls might_fault() which triggers an >>>>>>>>>>>>>>>>> ipipe_root_only() check. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> So, the question is, what is the best way of avoiding >>>>>>>>>>>>>>>>> this issue? Remove >>>>>>>>>>>>>>>>> the call to might_fault() when compiling with >>>>>>>>>>>>>>>>> CONFIG_IPIPE enabled? >>>>>>>>>>>>>>>>> Define __ipipe_safe_put_user which avoids the call to >>>>>>>>>>>>>>>>> might_fault() ? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Adjust might_fault() in a way that it only considers >>>>>>>>>>>>>>>> atomic non-root >>>>>>>>>>>>>>>> contexts as problematic. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> atomic meaning with hardware irqs off? or within an irq >>>>>>>>>>>>>>> handler? This >>>>>>>>>>>>>>> requires xenomai... >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> Hard irqs off || head domain stalled, I would say. >>>>>>>>>>>>> >>>>>>>>>>>>> ok, done, thanks. >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Actually, it does not work, the WARN_ON triggers in >>>>>>>>>>>> __rt_heap_bind for >>>>>>>>>>>> instance, which does some copies from user with hard irqs off. >>>>>>>>>>> >>>>>>>>>>> Hmm, isn't that a real bug? The nucleus should then try to >>>>>>>>>>> migrate to >>>>>>>>>>> Linux while the nklock is held - ouch... >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> In theory, the fault should be fixed up without requiring a >>>>>>>>>> migration. I >>>>>>>>>> will test this. >>>>>>>>> >>>>>>>>> In theory, user space could also provide an invalid pointer. >>>>>>>> >>>>>>>> >>>>>>>> Yes, this is what I am going to test, but an invalid pointer >>>>>>>> cause the >>>>>>>> fault to be fixed up so that the copy_from_user return -EFAULT. >>>>>>>> >>>>>>> >>>>>>> I don't think we fix-up over the head domain. Rather, the nucleus >>>>>>> receives the information that a fault has happened over the head >>>>>>> domain >>>>>>> and migrates to root. >>>>>> >>>>>> >>>>>> It depends on where the notification happens in the I-pipe patch. You >>>>>> could allow the kernel to fix-up the fault without notifying the >>>>>> nucleus. I thought I did that in the past, but apparently not in the >>>>>> latest patches as a segfault in rt_heap_bind causes a switch to >>>>>> secondary mode, but the nklock is automatically released as part >>>>>> of the >>>>>> migration to secondary mode. >>>>>> >>>>>> I am not sure the fix-up code is completely unsafe to be called >>>>>> over non >>>>>> root domain, as it only uses preempt_disable, preempt_enable, >>>>>> list_for_each_entry_rcu and only if the exception is found in a >>>>>> kernel >>>>>> module. >>>>>> >>>>> >>>>> Maybe. >>>>> >>>>> But why do we need to hold the nklock across the __rt_bind_helper in >>>>> __rt_queue_bind at all? Other binding services apparently don't have >>>>> this need. >>>> >>>> >>>> I suppose because we do not want the heap vanishing under our feet >>>> while >>>> we access its contents with the xnheap_* accessors. >>> >>> Right, but the general pattern for this is >>> >>> obj =3D xnregistry_fetch(handle) >>> get_nklock() >>> if (valid(obj)) >>> do_something_with(obj) >>> put_nklock() >> >> >> Seems rather to be get_nklock before xnregistry_fetch. >> >> > = > Yes. The other pattern is only used when the fetched handle shall be > revalidated before actual use. __rt_bind_helper does not revalidate, so > the handle is fetched under lock. We would have to re-revalidate the handle then - or leave __rt_bind_helper with nklock held so that the caller can follow up with more accesses to the object before releasing the lock again. It all boils down if we want to relax the constraints for using copy_to/from_user and allow faults inside IRQ-off sections. Jan -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 263 bytes Desc: OpenPGP digital signature URL: