Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 1 Feb 2022 17:29:42 +0000
From:   Daniel Thompson <daniel.thompson@linaro.org>
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     "Russell King (Oracle)" <linux@armlinux.org.uk>,
        Arnd Bergmann <arnd@arndb.de>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        linux-arch <linux-arch@vger.kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH v5 08/10] ARM: uaccess: add __{get,put}_kernel_nofault
Message-ID: <20220201172942.nxop6cjr3xfa4237@maple.lan>
References: <20210726141141.2839385-1-arnd@kernel.org>
 <20210726141141.2839385-9-arnd@kernel.org>
 <Yd8P37V/N9EkwmYq@wychelm>
 <Yd8ZEbywqjXkAx9k@shell.armlinux.org.uk>
 <20220113094754.6ei6ssiqbuw7tfj7@maple.lan>
 <CAK8P3a0=OkFcKbL+utDPTPf+RskFNdR8Vt-3BEWkO9g_FqSj5w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAK8P3a0=OkFcKbL+utDPTPf+RskFNdR8Vt-3BEWkO9g_FqSj5w@mail.gmail.com>
Precedence: bulk

On Thu, Jan 13, 2022 at 12:14:50PM +0100, Arnd Bergmann wrote:
> On Thu, Jan 13, 2022 at 10:47 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> > On Wed, Jan 12, 2022 at 06:08:17PM +0000, Russell King (Oracle) wrote:
> >
> > > 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.
> 
> Right, it seems this is the same as on x86.

Hmnn...

Looking a bit deeper into copy_from_kernel_nofault() there is an odd
asymmetry between copy_to_kernel_nofault(). Basically there is
copy_from_kernel_nofault_allowed() but no corresponding
copy_to_kernel_nofault_allowed() which means we cannot defend memory
pokes using a helper function.

I checked the behaviour of copy_to_kernel_nofault() on arm, arm64, mips,
powerpc, riscv, x86 kernels (which is pretty much everything where I
know how to fire up qemu). All except arm gracefully handle an
attempt to write to userspace (well, NULL actually) with
copy_to_kernel_nofault() so I think there still a few more changes
to fully fix this.

Looks like we would need a slightly more assertive change, either adding
a copy_to_kernel_nofault_allowed() or modifying the arm dabt handlers to
avoid faults on userspace access.

Any views on which is better?


Daniel.

> 
> > From f66a63b504ff582f261a506c54ceab8c0e77a98c Mon Sep 17 00:00:00 2001
> > From: Daniel Thompson <daniel.thompson@linaro.org>
> > 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@linaro.org>
> 
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>