On Thu, Sep 19, 2019 at 12:04:27AM +1000, Michael Ellerman wrote: > Michal Suchánek <msucha...@suse.de> writes: > > On Wed, 6 Mar 2019 12:10:38 +1100 > > Suraj Jitindar Singh <sjitindarsi...@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 <sjitindarsi...@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