* Sleeping in user_access section @ 2018-11-23 9:27 Julien Thierry 2018-11-23 9:57 ` hpa 0 siblings, 1 reply; 7+ messages in thread From: Julien Thierry @ 2018-11-23 9:27 UTC (permalink / raw) To: Peter Anvin, Ingo Molnar, LKML, linux-arm-kernel Cc: Catalin Marinas, Will Deacon, James Morse Hi, I made an attempt at implementing the user_access_begin()/user_access_end() macros along with the get/put_user_unsafe() for arm64 by toggling the status of PAN (more or less similar to x86's STAC/CTAC). With a small mistake in my patch, we realized that directly calling function that could reschedule while in a user_access section could lead to: - scheduling another task keeping the user_access status enabled despite the task never calling user_access_begin() - when re-scheduling the task that was mid user_access section, user_access would be disabled and the task would fault on the next get/put_user_unsafe. This is because __switch_to does not alter the user_access status when switching from next to prev (at least on arm64 we currently don't, and by looking at the x86 code I don't think this is done either). From my understanding, this is not an issue when the task in user_access mode gets scheduled out/in as a result of an interrupt as PAN and EFLAGS.AC get saved/restore on exception entry/exit (at least I know it is the case for PAN, I am less sure for the x86 side). So, the question is, should __switch_to take care of the user_access status when scheduling new tasks? Or should there be a restriction about scheduling out a task with user_access mode enabled and maybe add a warning if we can detect this? (Or did we miss something and this is not an issue on x86?) Thanks, -- Julien Thierry ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Sleeping in user_access section 2018-11-23 9:27 Sleeping in user_access section Julien Thierry @ 2018-11-23 9:57 ` hpa 2018-11-23 10:16 ` Julien Thierry 2018-11-23 10:50 ` Russell King - ARM Linux 0 siblings, 2 replies; 7+ messages in thread From: hpa @ 2018-11-23 9:57 UTC (permalink / raw) To: Julien Thierry, Ingo Molnar, LKML, linux-arm-kernel Cc: Catalin Marinas, Will Deacon, James Morse On November 23, 2018 1:27:02 AM PST, Julien Thierry <julien.thierry@arm.com> wrote: >Hi, > >I made an attempt at implementing the >user_access_begin()/user_access_end() macros along with the >get/put_user_unsafe() for arm64 by toggling the status of PAN (more or >less similar to x86's STAC/CTAC). > >With a small mistake in my patch, we realized that directly calling >function that could reschedule while in a user_access section could >lead to: > >- scheduling another task keeping the user_access status enabled >despite >the task never calling user_access_begin() > >- when re-scheduling the task that was mid user_access section, >user_access would be disabled and the task would fault on the next >get/put_user_unsafe. > > >This is because __switch_to does not alter the user_access status when >switching from next to prev (at least on arm64 we currently don't, and >by looking at the x86 code I don't think this is done either). > > > From my understanding, this is not an issue when the task in >user_access mode gets scheduled out/in as a result of an interrupt as >PAN and EFLAGS.AC get saved/restore on exception entry/exit (at least I > >know it is the case for PAN, I am less sure for the x86 side). > > >So, the question is, should __switch_to take care of the user_access >status when scheduling new tasks? Or should there be a restriction >about >scheduling out a task with user_access mode enabled and maybe add a >warning if we can detect this? > >(Or did we miss something and this is not an issue on x86?) > >Thanks, You should never call a sleeping function from a user_access section. It is intended for very limited regions. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Sleeping in user_access section 2018-11-23 9:57 ` hpa @ 2018-11-23 10:16 ` Julien Thierry 2018-11-23 10:50 ` Russell King - ARM Linux 1 sibling, 0 replies; 7+ messages in thread From: Julien Thierry @ 2018-11-23 10:16 UTC (permalink / raw) To: hpa, Ingo Molnar, LKML, linux-arm-kernel Cc: Catalin Marinas, Will Deacon, James Morse On 23/11/18 09:57, hpa@zytor.com wrote: > On November 23, 2018 1:27:02 AM PST, Julien Thierry <julien.thierry@arm.com> wrote: >> Hi, >> >> I made an attempt at implementing the >> user_access_begin()/user_access_end() macros along with the >> get/put_user_unsafe() for arm64 by toggling the status of PAN (more or >> less similar to x86's STAC/CTAC). >> >> With a small mistake in my patch, we realized that directly calling >> function that could reschedule while in a user_access section could >> lead to: >> >> - scheduling another task keeping the user_access status enabled >> despite >> the task never calling user_access_begin() >> >> - when re-scheduling the task that was mid user_access section, >> user_access would be disabled and the task would fault on the next >> get/put_user_unsafe. >> >> >> This is because __switch_to does not alter the user_access status when >> switching from next to prev (at least on arm64 we currently don't, and >> by looking at the x86 code I don't think this is done either). >> >> >> From my understanding, this is not an issue when the task in >> user_access mode gets scheduled out/in as a result of an interrupt as >> PAN and EFLAGS.AC get saved/restore on exception entry/exit (at least I >> >> know it is the case for PAN, I am less sure for the x86 side). >> >> >> So, the question is, should __switch_to take care of the user_access >> status when scheduling new tasks? Or should there be a restriction >> about >> scheduling out a task with user_access mode enabled and maybe add a >> warning if we can detect this? >> >> (Or did we miss something and this is not an issue on x86?) >> >> Thanks, > > You should never call a sleeping function from a user_access section. It is intended for very limited regions. > Thanks for the clarification. Would it be worth documenting this somewhere? And add a check to detect such issues? Also, those limited regions can be interrupted and preempted, but I guess you could consider the interrupted region being split into separate user_access regions, before and after the interrupt. Should it be stated that an exception/interrupt constitutes implicit user_access_end()/begin() when taken from/returning to a user_access region? Thanks, -- Julien Thierry ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Sleeping in user_access section 2018-11-23 9:57 ` hpa 2018-11-23 10:16 ` Julien Thierry @ 2018-11-23 10:50 ` Russell King - ARM Linux 2018-11-23 11:57 ` Julien Thierry 1 sibling, 1 reply; 7+ messages in thread From: Russell King - ARM Linux @ 2018-11-23 10:50 UTC (permalink / raw) To: hpa Cc: Julien Thierry, Ingo Molnar, LKML, linux-arm-kernel, Catalin Marinas, Will Deacon, James Morse On Fri, Nov 23, 2018 at 01:57:12AM -0800, hpa@zytor.com wrote: > You should never call a sleeping function from a user_access section. > It is intended for very limited regions. So, what happens if the "unsafe" user access causes a page fault that ends up sleeping? -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Sleeping in user_access section 2018-11-23 10:50 ` Russell King - ARM Linux @ 2018-11-23 11:57 ` Julien Thierry 2018-11-23 13:04 ` Russell King - ARM Linux 2018-11-27 19:05 ` H. Peter Anvin 0 siblings, 2 replies; 7+ messages in thread From: Julien Thierry @ 2018-11-23 11:57 UTC (permalink / raw) To: Russell King - ARM Linux, hpa Cc: Ingo Molnar, LKML, linux-arm-kernel, Catalin Marinas, Will Deacon, James Morse On 23/11/18 10:50, Russell King - ARM Linux wrote: > On Fri, Nov 23, 2018 at 01:57:12AM -0800, hpa@zytor.com wrote: >> You should never call a sleeping function from a user_access section. >> It is intended for very limited regions. > > So, what happens if the "unsafe" user access causes a page fault that > ends up sleeping? > Thanks for pointing that out. On the arm64 side, PAN state is saved in spsr and (if PAN feature is enabled in SCTLR) PAN bit gets set (disabling the user accesses). For software PAN we follow the same behaviour on exception entry. So upon exception we implicitly exit user_access mode and then re-enter it when returning from the exception. On x86, the EFLAGS.AC bit is also saved upon exception and I think it is cleared upon exception entry so there is implicit exit from the user_access mode when taking exception/interrupt. This however is just how those two architectures happen to behave and doesn't seem to be specified as part of the user_access API... Which is why I'd like to clarify the semantics of user_access region wrt sleeping functions. Thanks, -- Julien Thierry ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Sleeping in user_access section 2018-11-23 11:57 ` Julien Thierry @ 2018-11-23 13:04 ` Russell King - ARM Linux 2018-11-27 19:05 ` H. Peter Anvin 1 sibling, 0 replies; 7+ messages in thread From: Russell King - ARM Linux @ 2018-11-23 13:04 UTC (permalink / raw) To: Julien Thierry Cc: hpa, Ingo Molnar, LKML, linux-arm-kernel, Catalin Marinas, Will Deacon, James Morse On Fri, Nov 23, 2018 at 11:57:31AM +0000, Julien Thierry wrote: > > > On 23/11/18 10:50, Russell King - ARM Linux wrote: > >On Fri, Nov 23, 2018 at 01:57:12AM -0800, hpa@zytor.com wrote: > >>You should never call a sleeping function from a user_access section. > >>It is intended for very limited regions. > > > >So, what happens if the "unsafe" user access causes a page fault that > >ends up sleeping? > > > > Thanks for pointing that out. > > On the arm64 side, PAN state is saved in spsr and (if PAN feature is enabled > in SCTLR) PAN bit gets set (disabling the user accesses). For software PAN > we follow the same behaviour on exception entry. So upon exception we > implicitly exit user_access mode and then re-enter it when returning from > the exception. > > On x86, the EFLAGS.AC bit is also saved upon exception and I think it is > cleared upon exception entry so there is implicit exit from the user_access > mode when taking exception/interrupt. > > This however is just how those two architectures happen to behave and > doesn't seem to be specified as part of the user_access API... > > Which is why I'd like to clarify the semantics of user_access region wrt > sleeping functions. Explicitly calling a sleeping function may not be recommended, but it's a rather fundamental fact of the Linux kernel that if we want to access userspace, we must be either prepared to sleep, or fail the access and return an error. Looking at kernel/exit.c and kernel/compat.c, I see no sign of any retry mechanism in the current call sites, so any failed user access would return an error to userspace - which is not the behaviour that userspace would expect. It's possible that, when user_access_begin() et.al. are implemented, access_ok() also comes with the requirement to make sure that the userspace pages are accessible, but even _that_ would be racy, because there's no way to pin the pages, so another thread/CPU could page those user pages back out... leading to a fault. So, realistically, with how user_access_begin()..user_access_end() is currently being used, an architecture fundamentally has to be prepared for threads to sleep within that section of code through the action of the page fault handling. I can't see any other realistic possibility here. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Sleeping in user_access section 2018-11-23 11:57 ` Julien Thierry 2018-11-23 13:04 ` Russell King - ARM Linux @ 2018-11-27 19:05 ` H. Peter Anvin 1 sibling, 0 replies; 7+ messages in thread From: H. Peter Anvin @ 2018-11-27 19:05 UTC (permalink / raw) To: Julien Thierry, Russell King - ARM Linux Cc: Ingo Molnar, LKML, linux-arm-kernel, Catalin Marinas, Will Deacon, James Morse On 11/23/18 3:57 AM, Julien Thierry wrote: > > On x86, the EFLAGS.AC bit is also saved upon exception and I think it is > cleared upon exception entry so there is implicit exit from the > user_access mode when taking exception/interrupt. > No, it is restored, not cleared. In summary: on exceptions, user_access regions are suspended, and on return the user_access status is resumed. However, explicitly calling sleeping functions is not supported. -hpa ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-11-27 19:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-23 9:27 Sleeping in user_access section Julien Thierry 2018-11-23 9:57 ` hpa 2018-11-23 10:16 ` Julien Thierry 2018-11-23 10:50 ` Russell King - ARM Linux 2018-11-23 11:57 ` Julien Thierry 2018-11-23 13:04 ` Russell King - ARM Linux 2018-11-27 19:05 ` H. Peter Anvin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).