Christophe Leroy's on March 27, 2020 5:13 pm: > > > Le 27/03/2020 à 08:02, Nicholas Piggin a écrit : >> There is no need to allow user accesses when probing kernel addresses. >> >> Signed-off-by: Nicholas Piggin <npig...@gmail.com> >> --- >> arch/powerpc/include/asm/uaccess.h | 25 ++++++++++----- >> arch/powerpc/lib/Makefile | 2 +- >> arch/powerpc/lib/uaccess.c | 50 ++++++++++++++++++++++++++++++ >> 3 files changed, 68 insertions(+), 9 deletions(-) >> create mode 100644 arch/powerpc/lib/uaccess.c >> > > [...] > >> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile >> index b8de3be10eb4..a15060b5008e 100644 >> --- a/arch/powerpc/lib/Makefile >> +++ b/arch/powerpc/lib/Makefile >> @@ -36,7 +36,7 @@ extra-$(CONFIG_PPC64) += crtsavres.o >> endif >> >> obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \ >> - memcpy_power7.o >> + memcpy_power7.o uaccess.o > > Why only book3s/64 ? It applies to the 8xx and book3s/32 as well, I > think it should just be for all powerpc.
Okay I can do that. >> >> obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \ >> memcpy_64.o memcpy_mcsafe_64.o >> diff --git a/arch/powerpc/lib/uaccess.c b/arch/powerpc/lib/uaccess.c >> new file mode 100644 >> index 000000000000..0057ab52d6fe >> --- /dev/null >> +++ b/arch/powerpc/lib/uaccess.c >> @@ -0,0 +1,50 @@ >> +#include <linux/mm.h> >> +#include <linux/uaccess.h> >> + >> +static __always_inline long >> +probe_read_common(void *dst, const void __user *src, size_t size) >> +{ >> + long ret; >> + >> + pagefault_disable(); >> + ret = raw_copy_from_user_allowed(dst, src, size); >> + pagefault_enable(); >> + >> + return ret ? -EFAULT : 0; >> +} >> + >> +static __always_inline long >> +probe_write_common(void __user *dst, const void *src, size_t size) >> +{ >> + long ret; >> + >> + pagefault_disable(); >> + ret = raw_copy_to_user_allowed(dst, src, size); >> + pagefault_enable(); >> + >> + return ret ? -EFAULT : 0; >> +} >> + >> +long probe_kernel_read(void *dst, const void *src, size_t size) >> +{ >> + long ret; >> + mm_segment_t old_fs = get_fs(); >> + >> + set_fs(KERNEL_DS); >> + ret = probe_read_common(dst, (__force const void __user *)src, size); > > I think you should squash probe_read_common() here, having it separated > is a lot of lines for no added value. It also may make people believe it > overwrites the generic probe_read_common() Yeah I'll see how that looks. Thanks, Nick