On Wed, Jan 12, 2022 at 06:08:17PM +0000, Russell King (Oracle) wrote:
> On Wed, Jan 12, 2022 at 05:29:03PM +0000, Daniel Thompson wrote:
> > On Mon, Jul 26, 2021 at 04:11:39PM +0200, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@xxxxxxxx>
> > >
> > > These mimic the behavior of get_user and put_user, except
> > > for domain switching, address limit checking and handling
> > > of mismatched sizes, none of which are relevant here.
> > >
> > > To work with pre-Armv6 kernels, this has to avoid TUSER()
> > > inside of the new macros, the new approach passes the "t"
> > > string along with the opcode, which is a bit uglier but
> > > avoids duplicating more code.
> > >
> > > As there is no __get_user_asm_dword(), I work around it
> > > by copying 32 bit at a time, which is possible because
> > > the output size is known.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> >
> > I've just been bisecting some regressions running the kgdbts tests on
> > arm and this patch came up.
>
> So the software PAN code is working :)
Interesting. I noticed it was odd that kgdbts works just fine
if launched from kernel command line. I guess that runs before
PAN is activated. Neat.
> The kernel attempted to access an address that is in the userspace
> domain (NULL pointer) and took an exception.
>
> I suppose we should handle a domain fault more gracefully - what are
> the required semantics if the kernel attempts a userspace access
> using one of the _nofault() accessors?
I think the best answer might well be that, if the arch provides
implementations of hooks such as copy_from_kernel_nofault_allowed()
then the kernel should never attempt a userspace access using the
_nofault() accessors. That means they can do whatever they like!
In other words something like the patch below looks like a promising
approach.
Daniel.
>From f66a63b504ff582f261a506c54ceab8c0e77a98c Mon Sep 17 00:00:00 2001
From: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
Date: Thu, 13 Jan 2022 09:34:45 +0000
Subject: [PATCH] arm: mm: Implement copy_from_kernel_nofault_allowed()
Currently copy_from_kernel_nofault() can actually fault (due to software
PAN) if we attempt userspace access. In any case, the documented
behaviour for this function is to return -ERANGE if we attempt an access
outside of kernel space.
Implementing copy_from_kernel_nofault_allowed() solves both these
problems.
Signed-off-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
---
arch/arm/mm/Makefile | 2 +-
arch/arm/mm/maccess.c | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/mm/maccess.c
diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
index 3510503bc5e6..d1c5f4f256de 100644
--- a/arch/arm/mm/Makefile
+++ b/arch/arm/mm/Makefile
@@ -3,7 +3,7 @@
# Makefile for the linux arm-specific parts of the memory manager.
#
-obj-y := extable.o fault.o init.o iomap.o
+obj-y := extable.o fault.o init.o iomap.o maccess.o
obj-y += dma-mapping$(MMUEXT).o
obj-$(CONFIG_MMU) += fault-armv.o flush.o idmap.o ioremap.o \
mmap.o pgd.o mmu.o pageattr.o
diff --git a/arch/arm/mm/maccess.c b/arch/arm/mm/maccess.c
new file mode 100644
index 000000000000..0251062cb40d
--- /dev/null
+++ b/arch/arm/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;
+}
--
2.33.1
