[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 3 Jul 2020 17:14:30 +0100
From: Will Deacon <will@...nel.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
Catalin Marinas <catalin.marinas@....com>,
Sudeep Holla <sudeep.holla@....com>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>,
"Jason A . Donenfeld" <Jason@...c4.com>
Subject: Re: [RFC PATCH v2] arm64/acpi: disallow AML memory opregions to
access kernel memory
On Tue, Jun 23, 2020 at 06:32:25PM +0200, Ard Biesheuvel wrote:
> On Tue, 23 Jun 2020 at 18:27, Lorenzo Pieralisi
> <lorenzo.pieralisi@....com> wrote:
> > On Tue, Jun 23, 2020 at 11:37:55AM +0200, Ard Biesheuvel wrote:
> > > AML uses SystemMemory opregions to allow AML handlers to access MMIO
> > > registers of, e.g., GPIO controllers, or access reserved regions of
> > > memory that are owned by the firmware.
> > >
> > > Currently, we also allow AML access to memory that is owned by the
> > > kernel and mapped via the linear region, which does not seem to be
> > > supported by a valid use case, and exposes the kernel's internal
> > > state to AML methods that may be buggy and exploitable.
> > >
> > > On arm64, ACPI support requires booting in EFI mode, and so we can cross
> > > reference the requested region against the EFI memory map, rather than
> > > just do a minimal check on the first page. So let's only permit regions
> > > to be remapped by the ACPI core if
> > > - they don't appear in the EFI memory map at all (which is the case for
> > > most MMIO), or
> > > - they are covered by a single region in the EFI memory map, which is not
> > > of a type that describes memory that is given to the kernel at boot.
> > >
> > > Reported-by: Jason A. Donenfeld <Jason@...c4.com>
> > > Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> > > ---
> > > v2: do a more elaborate check on the region, against the EFI memory map
> > >
> > > arch/arm64/include/asm/acpi.h | 15 +---
> > > arch/arm64/kernel/acpi.c | 72 ++++++++++++++++++++
> > > 2 files changed, 73 insertions(+), 14 deletions(-)
[...]
> > > + case EFI_RUNTIME_SERVICES_CODE:
> > > + /*
> > > + * This would be unusual, but not problematic per se,
> > > + * as long as we take care not to create a writable
> > > + * mapping for executable code.
> > > + */
> > > + prot = PAGE_KERNEL_RO;
> >
> > Nit: IIUC this tweaks the current behaviour (so it is probably better
> > to move this change to another patch).
> >
>
> OK
>
> > Other than that the patch is sound and probably the best we could
> > do to harden the code, goes without saying - it requires testing.
> >
>
> Indeed. I will do some testing on the systems I have access to, and
> hopefully, other will as well.
Is this 5.9 material, or do you want it to go in as a fix?
Will
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
