[PATCH] ACPI: ioremap: avoid redundant rounding to OS page size

[Ard Biesheuvel]

On Mon, 17 Aug 2020 at 14:40, Christoph Hellwig <hch at infradead.org> wrote:
>
> On Mon, Aug 17, 2020 at 02:04:31PM +0200, Ard Biesheuvel wrote:
> > The arm64 implementation of acpi_os_ioremap() was recently updated to
> > tighten the checks around which parts of memory are permitted to be
> > mapped by ACPI code, which generally only needs access to memory regions
> > that are statically described by firmware, and any attempts to access
> > memory that is in active use by the OS is generally a bug or a hacking
> > attempt. This tightening is based on the EFI memory map, which describes
> > all memory in the system.
> >
> > The AArch64 architecture permits page sizes of 16k and 64k in addition
> > to the EFI default, which is 4k, which means that the EFI memory map may
> > describe regions that cannot be mapped seamlessly if the OS page size is
> > greater than 4k. This is usually not a problem, given that the EFI spec
> > does not permit memory regions requiring different memory attributes to
> > share a 64k page frame, and so the usual rounding to page size performed
> > by ioremap() is sufficient to deal with this. However, this rounding does
> > complicate our EFI memory map permission check, due to the loss of
> > information that occurs when several small regions share a single 64k
> > page frame (where rounding each of them will result in the same 64k
> > single page region).
> >
> > However, due to the fact that the region check occurs *before* the call
> > to ioremap() where the necessary rounding is performed, we can deal
> > with this issue simply by removing the redundant rounding performed by
> > acpi_os_map_iomem(), as it appears to be the only place where the
> > arguments to a call to acpi_os_ioremap() are rounded up. So omit the
> > rounding in the call, and instead, apply the necessary offset to the
> > result of kmap().
> >
> > Fixes: 1583052d111f ("arm64/acpi: disallow AML memory opregions to access kernel memory")
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
>
> To me the whole acpi_map() / acpi_os_map_iomem() concept looks bogus,
> especially as it mixes up iomem and RAM pages in a really bad way,
> and then throws in staic fixmap-like mappings as well.
>
> Also looking at the callers I see no point in keeping a list of the
> memory mappings.  Does anyone have an idea where this craziness comes
> from?  Which of the callers actually has to deal both with iomem and
> RAM mappings at the same time?
>
> It seems like we should be able to untangle the few callers and remove
> this mess entirely.

That would be my preference as well. However, this code is used by
x86, ia64 and arm64, and I'd like to get this piece in as a fix, given
that the referenced patch was merged in v5.9-rc1, and causes issues on
64k kernels running on older arm64 systems with not-quite-compliant
EFI firmware.