* [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user()
@ 2019-03-06 1:10 Suraj Jitindar Singh
2019-07-08 1:19 ` Michael Ellerman
2019-08-21 13:36 ` Michal Suchánek
0 siblings, 2 replies; 6+ messages in thread
From: Suraj Jitindar Singh @ 2019-03-06 1:10 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Suraj Jitindar Singh
Commit ddf35cf3764b ("powerpc: Use barrier_nospec in copy_from_user()")
Added barrier_nospec before loading from user-controller pointers.
The intention was to order the load from the potentially user-controlled
pointer vs a previous branch based on an access_ok() check or similar.
In order to achieve the same result, add a barrier_nospec to the
raw_copy_in_user() function before loading from such a user-controlled
pointer.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
arch/powerpc/include/asm/uaccess.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index e3a731793ea2..bb615592d5bb 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -306,6 +306,7 @@ extern unsigned long __copy_tofrom_user(void __user *to,
static inline unsigned long
raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
{
+ barrier_nospec();
return __copy_tofrom_user(to, from, n);
}
#endif /* __powerpc64__ */
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user()
2019-03-06 1:10 [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user() Suraj Jitindar Singh
@ 2019-07-08 1:19 ` Michael Ellerman
2019-08-21 13:36 ` Michal Suchánek
1 sibling, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2019-07-08 1:19 UTC (permalink / raw)
To: Suraj Jitindar Singh, linuxppc-dev; +Cc: Suraj Jitindar Singh
On Wed, 2019-03-06 at 01:10:38 UTC, Suraj Jitindar Singh wrote:
> Commit ddf35cf3764b ("powerpc: Use barrier_nospec in copy_from_user()")
> Added barrier_nospec before loading from user-controller pointers.
> The intention was to order the load from the potentially user-controlled
> pointer vs a previous branch based on an access_ok() check or similar.
>
> In order to achieve the same result, add a barrier_nospec to the
> raw_copy_in_user() function before loading from such a user-controlled
> pointer.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/6fbcdd59094ade30db63f32316e9502425d7b256
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user()
2019-03-06 1:10 [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user() Suraj Jitindar Singh
2019-07-08 1:19 ` Michael Ellerman
@ 2019-08-21 13:36 ` Michal Suchánek
2019-09-02 8:47 ` [PATCH] Revert "powerpc: Add barrier_nospec to raw_copy_in_user()" Michal Suchanek
2019-09-18 14:04 ` [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user() Michael Ellerman
1 sibling, 2 replies; 6+ messages in thread
From: Michal Suchánek @ 2019-08-21 13:36 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: linuxppc-dev, Al Viro
On Wed, 6 Mar 2019 12:10:38 +1100
Suraj Jitindar Singh <sjitindarsingh@gmail.com> wrote:
> Commit ddf35cf3764b ("powerpc: Use barrier_nospec in copy_from_user()")
> Added barrier_nospec before loading from user-controller pointers.
> The intention was to order the load from the potentially user-controlled
> pointer vs a previous branch based on an access_ok() check or similar.
>
> In order to achieve the same result, add a barrier_nospec to the
> raw_copy_in_user() function before loading from such a user-controlled
> pointer.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
> arch/powerpc/include/asm/uaccess.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index e3a731793ea2..bb615592d5bb 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -306,6 +306,7 @@ extern unsigned long __copy_tofrom_user(void __user *to,
> static inline unsigned long
> raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
> {
> + barrier_nospec();
> return __copy_tofrom_user(to, from, n);
> }
> #endif /* __powerpc64__ */
Hello,
AFAICT this is not needed. The data cannot be used in the kernel without
subsequent copy_from_user which already has the nospec barrier.
Thanks
Michal
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] Revert "powerpc: Add barrier_nospec to raw_copy_in_user()"
2019-08-21 13:36 ` Michal Suchánek
@ 2019-09-02 8:47 ` Michal Suchanek
2019-09-18 14:04 ` [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user() Michael Ellerman
1 sibling, 0 replies; 6+ messages in thread
From: Michal Suchanek @ 2019-09-02 8:47 UTC (permalink / raw)
To: linuxppc-dev, Suraj Jitindar Singh
Cc: Mathieu Malaterre, linux-kernel, Paul Mackerras, Anton Blanchard,
Michal Suchanek
This reverts commit 6fbcdd59094ade30db63f32316e9502425d7b256.
Not needed. Data handled by raw_copy_in_user must be loaded through
copy_from_user to be used in the kernel which already has the barrier.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
arch/powerpc/include/asm/uaccess.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 8b03eb44e876..76f34346b642 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -312,7 +312,6 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
{
unsigned long ret;
- barrier_nospec();
allow_user_access(to, from, n);
ret = __copy_tofrom_user(to, from, n);
prevent_user_access(to, from, n);
--
2.22.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user()
2019-08-21 13:36 ` Michal Suchánek
2019-09-02 8:47 ` [PATCH] Revert "powerpc: Add barrier_nospec to raw_copy_in_user()" Michal Suchanek
@ 2019-09-18 14:04 ` Michael Ellerman
2019-12-16 6:05 ` Michal Suchánek
1 sibling, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2019-09-18 14:04 UTC (permalink / raw)
To: Michal Suchánek, Suraj Jitindar Singh; +Cc: linuxppc-dev, Al Viro
Michal Suchánek <msuchanek@suse.de> writes:
> On Wed, 6 Mar 2019 12:10:38 +1100
> Suraj Jitindar Singh <sjitindarsingh@gmail.com> wrote:
>
>> Commit ddf35cf3764b ("powerpc: Use barrier_nospec in copy_from_user()")
>> Added barrier_nospec before loading from user-controller pointers.
>> The intention was to order the load from the potentially user-controlled
>> pointer vs a previous branch based on an access_ok() check or similar.
>>
>> In order to achieve the same result, add a barrier_nospec to the
>> raw_copy_in_user() function before loading from such a user-controlled
>> pointer.
>>
>> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>> ---
>> arch/powerpc/include/asm/uaccess.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
>> index e3a731793ea2..bb615592d5bb 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -306,6 +306,7 @@ extern unsigned long __copy_tofrom_user(void __user *to,
>> static inline unsigned long
>> raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
>> {
>> + barrier_nospec();
>> return __copy_tofrom_user(to, from, n);
>> }
>> #endif /* __powerpc64__ */
>
> Hello,
>
> AFAICT this is not needed. The data cannot be used in the kernel without
> subsequent copy_from_user which already has the nospec barrier.
Suraj did talk to me about this before sending the patch. My memory is
that he came up with a scenario where it was at least theoretically
useful, but he didn't mention that in the change log. He was working on
nested KVM at the time.
I've now forgotten, and he's moved on to other things so probably won't
remember either, if he's even checking his mail :/
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user()
2019-09-18 14:04 ` [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user() Michael Ellerman
@ 2019-12-16 6:05 ` Michal Suchánek
0 siblings, 0 replies; 6+ messages in thread
From: Michal Suchánek @ 2019-12-16 6:05 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Al Viro, Suraj Jitindar Singh
On Thu, Sep 19, 2019 at 12:04:27AM +1000, Michael Ellerman wrote:
> Michal Suchánek <msuchanek@suse.de> writes:
> > On Wed, 6 Mar 2019 12:10:38 +1100
> > Suraj Jitindar Singh <sjitindarsingh@gmail.com> wrote:
> >
> >> Commit ddf35cf3764b ("powerpc: Use barrier_nospec in copy_from_user()")
> >> Added barrier_nospec before loading from user-controller pointers.
> >> The intention was to order the load from the potentially user-controlled
> >> pointer vs a previous branch based on an access_ok() check or similar.
> >>
> >> In order to achieve the same result, add a barrier_nospec to the
> >> raw_copy_in_user() function before loading from such a user-controlled
> >> pointer.
> >>
> >> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> >> ---
> >> arch/powerpc/include/asm/uaccess.h | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> >> index e3a731793ea2..bb615592d5bb 100644
> >> --- a/arch/powerpc/include/asm/uaccess.h
> >> +++ b/arch/powerpc/include/asm/uaccess.h
> >> @@ -306,6 +306,7 @@ extern unsigned long __copy_tofrom_user(void __user *to,
> >> static inline unsigned long
> >> raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
> >> {
> >> + barrier_nospec();
> >> return __copy_tofrom_user(to, from, n);
> >> }
> >> #endif /* __powerpc64__ */
> >
> > Hello,
> >
> > AFAICT this is not needed. The data cannot be used in the kernel without
> > subsequent copy_from_user which already has the nospec barrier.
>
> Suraj did talk to me about this before sending the patch. My memory is
> that he came up with a scenario where it was at least theoretically
> useful, but he didn't mention that in the change log. He was working on
> nested KVM at the time.
>
> I've now forgotten, and he's moved on to other things so probably won't
> remember either, if he's even checking his mail :/
In absence of any argument for the code and absence of the same code on
other architectures I would say you were just confused when merging
this. The code is confusing after all.
Thanks
Michal
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-16 6:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-06 1:10 [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user() Suraj Jitindar Singh
2019-07-08 1:19 ` Michael Ellerman
2019-08-21 13:36 ` Michal Suchánek
2019-09-02 8:47 ` [PATCH] Revert "powerpc: Add barrier_nospec to raw_copy_in_user()" Michal Suchanek
2019-09-18 14:04 ` [PATCH] powerpc: Add barrier_nospec to raw_copy_in_user() Michael Ellerman
2019-12-16 6:05 ` Michal Suchánek
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).