[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMj1kXFX5okSWZ8SgnOrbs7qVSS1A2kyL12UXQi8JcEWmcDb=w@mail.gmail.com>
Date: Tue, 23 Jun 2020 10:16:19 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Will Deacon <will@...nel.org>
Cc: Linux ARM <linux-arm-kernel@...ts.infradead.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>, Catalin Marinas <catalin.marinas@....com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>, Sudeep Holla <sudeep.holla@....com>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>,
"Jason A . Donenfeld" <Jason@...c4.com>
Subject: Re: [RFC PATCH] arm64/acpi: disallow AML memory opregions to access
kernel memory
On Tue, 23 Jun 2020 at 10:13, Will Deacon <will@...nel.org> wrote:
>
> On Mon, Jun 22, 2020 at 11:27:19AM +0200, Ard Biesheuvel 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 wonder if it would be better to poison this so that if we do see reports
> of AML crashes we'll know straight away that it tried to access memory
> mapped by the linear region, as opposed to some other NULL dereference.
>
We could just add a WARN_ONCE() here, no?
> Anyway, no objections to the idea. Be good for some of the usual ACPI
> suspects to check this doesn't blow up immediately, though.
>
Indeed, hence the RFC. Jason does have a point regarding the range
check, so I will try to do something about that and send a v2.
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.
