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

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

[Ard Biesheuvel]

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@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;
-- 
2.17.1

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

[Ard Biesheuvel]

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@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;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [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@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@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!

Re: [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@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@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!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel