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

[Rafael J. Wysocki]

On Tue, Aug 18, 2020 at 11:14 AM Ard Biesheuvel <ardb at kernel.org> 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 masking when
> assigning the map->virt member.
>
> Fixes: 1583052d111f ("arm64/acpi: disallow AML memory opregions to access kernel memory")
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
> v2: return the correct virtual address for hits in the cached mappings array
>
>  drivers/acpi/osl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 6ad8cb05f672..acf6abc693a0 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -350,7 +350,7 @@ void __iomem __ref
>
>         pg_off = round_down(phys, PAGE_SIZE);
>         pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
> -       virt = acpi_map(pg_off, pg_sz);
> +       virt = acpi_map(phys, size);
>         if (!virt) {
>                 mutex_unlock(&acpi_ioremap_lock);
>                 kfree(map);
> @@ -358,7 +358,7 @@ void __iomem __ref
>         }
>
>         INIT_LIST_HEAD(&map->list);
> -       map->virt = virt;
> +       map->virt = (void __iomem __force *)((unsigned long)virt & PAGE_MASK);
>         map->phys = pg_off;
>         map->size = pg_sz;
>         map->track.refcount = 1;
> --

Applied as 5.9-rc material, thanks!