| Message ID | 20211221134242.98877-2-kernelfans@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] arm64: uaccess: disable preempt during uaccess through ttbr0 | expand |
On Tue, Dec 21, 2021 at 09:42:42PM +0800, Pingfan Liu wrote: > For the pair of > uaccess_enable_privileged()/uaccess_disable_privileged(), if preemption > happens, the exposure of the access to user space by another code piece > is unexpected. How can this happen? We set PAN on exception entry. > For the pair of > __uaccess_enable_tco_async()/__uaccess_disable_tco_async(), the change > of PSTATE.TCO is exposed unexpectedly. As above, we disable TCO on exception entry. > For the two cases, simply disable preemption to fix the issue. > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > Cc: Andrey Konovalov <andreyknvl@gmail.com> > Cc: Arnd Bergmann <arnd@arndb.de> > To: linux-arm-kernel@lists.infradead.org > --- > arch/arm64/include/asm/uaccess.h | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h > index 406888877bbd..37d5531f210d 100644 > --- a/arch/arm64/include/asm/uaccess.h > +++ b/arch/arm64/include/asm/uaccess.h > @@ -193,14 +193,18 @@ static inline void __uaccess_enable_tco(void) > */ > static inline void __uaccess_disable_tco_async(void) > { > - if (system_uses_mte_async_or_asymm_mode()) > - __uaccess_disable_tco(); > + if (system_uses_mte_async_or_asymm_mode()) { > + __uaccess_disable_tco(); > + preempt_enable(); > + } > } > > static inline void __uaccess_enable_tco_async(void) > { > - if (system_uses_mte_async_or_asymm_mode()) > + if (system_uses_mte_async_or_asymm_mode()) { > + preempt_disable(); > __uaccess_enable_tco(); > + } > } As with the previous patch, disabling preemption prior to a might_sleep() is wrong.
On Wed, Dec 22, 2021 at 01:02:44PM +0000, Catalin Marinas wrote: > On Tue, Dec 21, 2021 at 09:42:42PM +0800, Pingfan Liu wrote: > > For the pair of > > uaccess_enable_privileged()/uaccess_disable_privileged(), if preemption > > happens, the exposure of the access to user space by another code piece > > is unexpected. > > How can this happen? We set PAN on exception entry. > Yes, I make the some mistake as [1/2] > > For the pair of > > __uaccess_enable_tco_async()/__uaccess_disable_tco_async(), the change > > of PSTATE.TCO is exposed unexpectedly. > > As above, we disable TCO on exception entry. > Yes. > > For the two cases, simply disable preemption to fix the issue. > > > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > > Cc: Andrey Konovalov <andreyknvl@gmail.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > To: linux-arm-kernel@lists.infradead.org > > --- > > arch/arm64/include/asm/uaccess.h | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h > > index 406888877bbd..37d5531f210d 100644 > > --- a/arch/arm64/include/asm/uaccess.h > > +++ b/arch/arm64/include/asm/uaccess.h > > @@ -193,14 +193,18 @@ static inline void __uaccess_enable_tco(void) > > */ > > static inline void __uaccess_disable_tco_async(void) > > { > > - if (system_uses_mte_async_or_asymm_mode()) > > - __uaccess_disable_tco(); > > + if (system_uses_mte_async_or_asymm_mode()) { > > + __uaccess_disable_tco(); > > + preempt_enable(); > > + } > > } > > > > static inline void __uaccess_enable_tco_async(void) > > { > > - if (system_uses_mte_async_or_asymm_mode()) > > + if (system_uses_mte_async_or_asymm_mode()) { > > + preempt_disable(); > > __uaccess_enable_tco(); > > + } > > } > > As with the previous patch, disabling preemption prior to a > might_sleep() is wrong. > Right. I will make clear the kernel_{entry/_exit} part. Appreciate for your help again. Thanks, Pingfan
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index 406888877bbd..37d5531f210d 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -193,14 +193,18 @@ static inline void __uaccess_enable_tco(void) */ static inline void __uaccess_disable_tco_async(void) { - if (system_uses_mte_async_or_asymm_mode()) - __uaccess_disable_tco(); + if (system_uses_mte_async_or_asymm_mode()) { + __uaccess_disable_tco(); + preempt_enable(); + } } static inline void __uaccess_enable_tco_async(void) { - if (system_uses_mte_async_or_asymm_mode()) + if (system_uses_mte_async_or_asymm_mode()) { + preempt_disable(); __uaccess_enable_tco(); + } } static inline void uaccess_disable_privileged(void) @@ -208,13 +212,16 @@ static inline void uaccess_disable_privileged(void) __uaccess_disable_tco(); if (uaccess_ttbr0_disable()) - return; + goto out; __uaccess_enable_hw_pan(); +out: + preempt_enable(); } static inline void uaccess_enable_privileged(void) { + preempt_disable(); __uaccess_enable_tco(); if (uaccess_ttbr0_enable())
For the pair of uaccess_enable_privileged()/uaccess_disable_privileged(), if preemption happens, the exposure of the access to user space by another code piece is unexpected. For the pair of __uaccess_enable_tco_async()/__uaccess_disable_tco_async(), the change of PSTATE.TCO is exposed unexpectedly. For the two cases, simply disable preemption to fix the issue. Signed-off-by: Pingfan Liu <kernelfans@gmail.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> Cc: Andrey Konovalov <andreyknvl@gmail.com> Cc: Arnd Bergmann <arnd@arndb.de> To: linux-arm-kernel@lists.infradead.org --- arch/arm64/include/asm/uaccess.h | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)