[RFC PATCH v2] arm64/acpi: disallow AML memory opregions to access kernel memory

[Will Deacon]

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 at arm.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 at zx2c4.com>
> > > Signed-off-by: Ard Biesheuvel <ardb at kernel.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