On Sat, Dec 05, 2020 at 08:43:06AM +0000, Christophe Leroy wrote:
> Since commit c33165253492 ("powerpc: use non-set_fs based maccess
> routines"), userspace access is not granted anymore when using
> copy_from_kernel_nofault()
>
> However, kthread_probe_data() uses copy_from_kernel_nofault()
> to check validity of pointers. When the pointer is NULL,
> it points to userspace, leading to a KUAP fault and triggering
> the following big hammer warning many times when you request
> a sysrq "show task":
> To avoid that, copy_from_kernel_nofault_allowed() is used to check
> whether the address is a valid kernel address. But the default
> version of it returns true for any address.
>
> Provide a powerpc version of copy_from_kernel_nofault_allowed()
> that returns false when the address is below TASK_USER_MAX,
> so that copy_from_kernel_nofault() will return -ERANGE.
Looks good. I wonder if we should just default to the TASK_SIZE_MAX
check in copy_from_kernel_nofault_allowed for architectures that select
CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE?
>
> Reported-by: Qian Cai <qcai@xxxxxxxxxx>
> Fixes: c33165253492 ("powerpc: use non-set_fs based maccess routines")
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> ---
> This issue was introduced in 5.10. I didn't mark it for stable, hopping it will go into 5.10-rc7
> ---
> arch/powerpc/mm/Makefile | 2 +-
> arch/powerpc/mm/maccess.c | 9 +++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/mm/maccess.c
>
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index 5e147986400d..55b4a8bd408a 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -5,7 +5,7 @@
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y := fault.o mem.o pgtable.o mmap.o \
> +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> init_$(BITS).o pgtable_$(BITS).o \
> pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> init-common.o mmu_context.o drmem.o
> diff --git a/arch/powerpc/mm/maccess.c b/arch/powerpc/mm/maccess.c
> new file mode 100644
> index 000000000000..56e97c0fb233
> --- /dev/null
> +++ b/arch/powerpc/mm/maccess.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/uaccess.h>
> +#include <linux/kernel.h>
> +
> +bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
> +{
> + return (unsigned long)unsafe_src >= TASK_SIZE_MAX;
> +}
> --
> 2.25.0
---end quoted text---
