Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHmME9oNwDra2Vi+jsy4YZ81HVygyyRXTJeni58CaJqOmfmepA@mail.gmail.com>
Date: Mon, 22 Jun 2020 15:09:28 -0600
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>, 
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>, Will Deacon <will@...nel.org>, 
	Catalin Marinas <catalin.marinas@....com>, lorenzo.pieralisi@....com, sudeep.holla@....com, 
	Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [RFC PATCH] arm64/acpi: disallow AML memory opregions to access
 kernel memory

On Mon, Jun 22, 2020 at 3:27 AM Ard Biesheuvel <ardb@...nel.org> wrote:
>
> ACPI provides support for SystemMemory opregions, to allow AML methods
> to access MMIO registers of, e.g., GPIO controllers, or access reserved
> regions of memory that are owned by the firmware.
>
> Currently, we also permit AML methods to access 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.
>
> So close the door on this, and simply reject AML remapping requests for
> any memory that has a valid mapping in the linear region.
>
> Reported-by: Jason A. Donenfeld <Jason@...c4.com>
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> ---
>  arch/arm64/include/asm/acpi.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index a45366c3909b..18dcef4e6764 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -50,9 +50,9 @@ pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
>  static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
>                                             acpi_size size)
>  {
> -       /* For normal memory we already have a cacheable mapping. */
> +       /* Don't allow access to kernel memory from AML code */
>         if (memblock_is_map_memory(phys))
> -               return (void __iomem *)__phys_to_virt(phys);
> +               return NULL;

I'm happy to see that implementation-wise it's so easy. Take my
Acked-by, but I'd really prefer somebody with some ACPI experience and
has looked at tons of DSDTs over the years to say whether or not this
will break hardware.

[As an aside, the current implementation is actually "wrong", since
that will trap when an ASL tries to write to regions mapped as
read-only, which shouldn't happen when selecting physical addresses. I
learned this the ~hard way when writing those exploits last week. :-P]

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.