Re: [PATCH 1/1] ACPI: fix acpi table use after free

[Mike Rapoport]

On Tue, Mar 09, 2021 at 07:29:51PM +0100, Rafael J. Wysocki wrote:
> On Tue, Mar 9, 2021 at 6:54 PM Mike Rapoport <rppt@xxxxxxxxxxxxx> wrote:
> >
> > On Sun, Mar 07, 2021 at 09:46:22AM +0200, Mike Rapoport wrote:
> > > Hello Rafael,
> > >
> > > On Fri, Mar 05, 2021 at 02:30:07PM +0100, Rafael J. Wysocki wrote:
> > > > On Fri, Mar 5, 2021 at 12:14 AM George Kennedy <george.kennedy@xxxxxxxxxx> wrote:
> > > >
> > > > > The ibft table, for example, is mapped in via acpi_map() and kmap(). The
> > > > > page for the ibft table is not reserved, so it can end up on the freelist.
> > > >
> > > > You appear to be saying that it is not sufficient to kmap() a page in
> > > > order to use it safely.  It is also necessary to reserve it upfront,
> > > > for example with the help of memblock_reserve().  Is that correct?  If
> > > > so, is there an alternative way to reserve a page frame?
> > >
> > > Like David said in the other reply, if a BIOS does not mark the memory that
> > > contains an ACPI table as used (e.g. reserved or ACPI data), we need to
> > > make sure the kernel knows that such memory is in use and an early call to
> > > memblock_reserve() is exactly what we need here.
> > > George had this issue with iBFT, but in general this could be any table
> > > that a buggy BIOS forgot to mark as ACPI data.
> >
> > BTW, I wonder is there a fundamental reason to use ioremap() to access ACPI
> > tables at all?
> > In the end, they reside in RAM and, apparently, they live at the same DIMM
> > as neighboring "normal memory" so why cannot we just map them normally as
> > read-only not executable?
> 
> This may be NVS memory (depending on the configuration of the system)
> which isn't "normal" RAM AFAICS.

Hmm, according to the description of "ACPI NVS" in ACPI 6.3

	ACPI NVS Memory. This range of addresses is in use or reserved by
			 the system and must not be used by the operating
			 system. This range is required to be saved and
 			 restored across an NVS sleep.

it behaves more like "normal" RAM rather than actual non-volatile storage.

There are other places in ACPI text that imply that "ACPI NVS" is actually
RAM, it's just reserved by the firmware.

And judging by the example below both "ACPI data" and "ACPI NVS" live in
the very same DIMM as "usable" RAM.

[    0.000000] BIOS-e820: [mem 0x0000000029931000-0x0000000029932fff] usable
[    0.000000] BIOS-e820: [mem 0x0000000029933000-0x000000002993afff] ACPI data
[    0.000000] BIOS-e820: [mem 0x000000002993b000-0x000000002993bfff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x000000002993c000-0x0000000029940fff] ACPI data
[    0.000000] BIOS-e820: [mem 0x0000000029941000-0x0000000029944fff] usable

Unfortunately, both UEFI and ACPI standards are very vague about the
meaning of "ACPI NVS" so there may be systems that use real non-volatile
storage for it...

-- 
Sincerely yours,
Mike.