| Message ID | 20240614140149.1654251-1-Raju.Rangoju@amd.com |
|---|---|
| State | Accepted |
| Commit | a83e1385b780d41307433ddbc86e3c528db031f0 |
| Headers | show |
| Series | ACPICA: Revert "ACPICA: avoid Info: mapping multiple BARs. Your kernel is fine." | expand |
On Fri, Jun 14, 2024 at 4:02 PM Raju Rangoju <Raju.Rangoju@amd.com> wrote: > > Undo the modifications made in commit d410ee5109a1 ("ACPICA: avoid > "Info: mapping multiple BARs. Your kernel is fine.""). The initial > purpose of this commit was to stop memory mappings for operation > regions from overlapping page boundaries, as it can trigger warnings > if different page attributes are present. > > However, it was found that when this situation arises, mapping > continues until the boundary's end, but there is still an attempt to > read/write the entire length of the map, leading to a NULL pointer > deference. For example, if a four-byte mapping request is made but > only one byte is mapped because it hits the current page boundary's > end, a four-byte read/write attempt is still made, resulting in a NULL > pointer deference. > > Instead, map the entire length, as the ACPI specification does not > mandate that it must be within the same page boundary. It is > permissible for it to be mapped across different regions. > > Link: https://github.com/acpica/acpica/pull/954 > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218849 > Fixes: d410ee5109a1 ("ACPICA: avoid "Info: mapping multiple BARs. Your kernel is fine."") > Co-developed-by: Sanath S <Sanath.S@amd.com> > Signed-off-by: Sanath S <Sanath.S@amd.com> > Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com> > --- > drivers/acpi/acpica/exregion.c | 23 ++--------------------- > 1 file changed, 2 insertions(+), 21 deletions(-) > > diff --git a/drivers/acpi/acpica/exregion.c b/drivers/acpi/acpica/exregion.c > index 8907b8bf4267..c49b9f8de723 100644 > --- a/drivers/acpi/acpica/exregion.c > +++ b/drivers/acpi/acpica/exregion.c > @@ -44,7 +44,6 @@ acpi_ex_system_memory_space_handler(u32 function, > struct acpi_mem_mapping *mm = mem_info->cur_mm; > u32 length; > acpi_size map_length; > - acpi_size page_boundary_map_length; > #ifdef ACPI_MISALIGNMENT_NOT_SUPPORTED > u32 remainder; > #endif > @@ -138,26 +137,8 @@ acpi_ex_system_memory_space_handler(u32 function, > map_length = (acpi_size) > ((mem_info->address + mem_info->length) - address); > > - /* > - * If mapping the entire remaining portion of the region will cross > - * a page boundary, just map up to the page boundary, do not cross. > - * On some systems, crossing a page boundary while mapping regions > - * can cause warnings if the pages have different attributes > - * due to resource management. > - * > - * This has the added benefit of constraining a single mapping to > - * one page, which is similar to the original code that used a 4k > - * maximum window. > - */ > - page_boundary_map_length = (acpi_size) > - (ACPI_ROUND_UP(address, ACPI_DEFAULT_PAGE_SIZE) - address); > - if (page_boundary_map_length == 0) { > - page_boundary_map_length = ACPI_DEFAULT_PAGE_SIZE; > - } > - > - if (map_length > page_boundary_map_length) { > - map_length = page_boundary_map_length; > - } > + if (map_length > ACPI_DEFAULT_PAGE_SIZE) > + map_length = ACPI_DEFAULT_PAGE_SIZE; > > /* Create a new mapping starting at the address given */ > > -- Applied as 6.10-rc material, thanks!
diff --git a/drivers/acpi/acpica/exregion.c b/drivers/acpi/acpica/exregion.c index 8907b8bf4267..c49b9f8de723 100644 --- a/drivers/acpi/acpica/exregion.c +++ b/drivers/acpi/acpica/exregion.c @@ -44,7 +44,6 @@ acpi_ex_system_memory_space_handler(u32 function, struct acpi_mem_mapping *mm = mem_info->cur_mm; u32 length; acpi_size map_length; - acpi_size page_boundary_map_length; #ifdef ACPI_MISALIGNMENT_NOT_SUPPORTED u32 remainder; #endif @@ -138,26 +137,8 @@ acpi_ex_system_memory_space_handler(u32 function, map_length = (acpi_size) ((mem_info->address + mem_info->length) - address); - /* - * If mapping the entire remaining portion of the region will cross - * a page boundary, just map up to the page boundary, do not cross. - * On some systems, crossing a page boundary while mapping regions - * can cause warnings if the pages have different attributes - * due to resource management. - * - * This has the added benefit of constraining a single mapping to - * one page, which is similar to the original code that used a 4k - * maximum window. - */ - page_boundary_map_length = (acpi_size) - (ACPI_ROUND_UP(address, ACPI_DEFAULT_PAGE_SIZE) - address); - if (page_boundary_map_length == 0) { - page_boundary_map_length = ACPI_DEFAULT_PAGE_SIZE; - } - - if (map_length > page_boundary_map_length) { - map_length = page_boundary_map_length; - } + if (map_length > ACPI_DEFAULT_PAGE_SIZE) + map_length = ACPI_DEFAULT_PAGE_SIZE; /* Create a new mapping starting at the address given */
Undo the modifications made in commit d410ee5109a1 ("ACPICA: avoid "Info: mapping multiple BARs. Your kernel is fine.""). The initial purpose of this commit was to stop memory mappings for operation regions from overlapping page boundaries, as it can trigger warnings if different page attributes are present. However, it was found that when this situation arises, mapping continues until the boundary's end, but there is still an attempt to read/write the entire length of the map, leading to a NULL pointer deference. For example, if a four-byte mapping request is made but only one byte is mapped because it hits the current page boundary's end, a four-byte read/write attempt is still made, resulting in a NULL pointer deference. Instead, map the entire length, as the ACPI specification does not mandate that it must be within the same page boundary. It is permissible for it to be mapped across different regions. Link: https://github.com/acpica/acpica/pull/954 Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218849 Fixes: d410ee5109a1 ("ACPICA: avoid "Info: mapping multiple BARs. Your kernel is fine."") Co-developed-by: Sanath S <Sanath.S@amd.com> Signed-off-by: Sanath S <Sanath.S@amd.com> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com> --- drivers/acpi/acpica/exregion.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-)