Dear Rafael, On 7/26/23 23:35, Rafael J. Wysocki wrote: > On Wed, Jul 26, 2023 at 6:53 AM Kiwamu Okabe <okabe@xxxxxxxxxxxxx> wrote: >> >> The Infer static analyzer https://fbinfer.com/ reports following >> NULL poinster dereference by the acpi_os_map_memory() function. >> I believe this patch does fix the issue without any panic. > > Please demonstrate to me that the NULL pointer dereference can > actually happen in this code. The `acpi_table_initrd_override()` function potentially occurs NULL pointer dereference on `table->length`, ``` while (table_offset + ACPI_HEADER_SIZE <= all_tables_size) { table = acpi_os_map_memory(acpi_tables_addr + table_offset, ACPI_HEADER_SIZE); if (table_offset + table->length > all_tables_size) { acpi_os_unmap_memory(table, ACPI_HEADER_SIZE); WARN_ON(1); return AE_OK; } ``` because the acpi_os_map_memory() function potentially returns NULL, ``` void __iomem __ref *acpi_os_map_iomem(acpi_physical_address phys, acpi_size size) { --snip-- map = kzalloc(sizeof(*map), GFP_KERNEL); if (!map) { mutex_unlock(&acpi_ioremap_lock); return NULL; } --snip-- void *__ref acpi_os_map_memory(acpi_physical_address phys, acpi_size size) { return (void *)acpi_os_map_iomem(phys, size); } ``` because the `kzalloc()` potentially returns NULL. And also, the other code have NULL check to call `acpi_os_map_memory()` as following. ``` subtable_header = acpi_os_map_memory(address, sizeof(*subtable_header)); if (!subtable_header) return -ENOMEM; --snip-- rsdp = acpi_os_map_memory(rsdp_address, sizeof(struct acpi_table_rsdp)); if (!rsdp) { return_ACPI_STATUS(AE_NO_MEMORY); } ``` >> Signed-off-by: Kiwamu Okabe <okabe@xxxxxxxxxxxxx> >> --- >> drivers/acpi/tables.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c >> index 8ab0a82b4da4..ae7b7343bacf 100644 >> --- a/drivers/acpi/tables.c >> +++ b/drivers/acpi/tables.c >> @@ -717,6 +717,9 @@ acpi_table_initrd_override(struct acpi_table_header *existing_table, >> while (table_offset + ACPI_HEADER_SIZE <= all_tables_size) { >> table = acpi_os_map_memory(acpi_tables_addr + table_offset, >> ACPI_HEADER_SIZE); >> + if (WARN_ON(!table)) { >> + return AE_OK; >> + } >> if (table_offset + table->length > all_tables_size) { >> acpi_os_unmap_memory(table, ACPI_HEADER_SIZE); >> WARN_ON(1); >> @@ -772,6 +775,9 @@ static void __init acpi_table_initrd_scan(void) >> while (table_offset + ACPI_HEADER_SIZE <= all_tables_size) { >> table = acpi_os_map_memory(acpi_tables_addr + table_offset, >> ACPI_HEADER_SIZE); >> + if (WARN_ON(!table)) { >> + return; >> + } >> if (table_offset + table->length > all_tables_size) { >> acpi_os_unmap_memory(table, ACPI_HEADER_SIZE); >> WARN_ON(1); >> -- >> 2.39.2 >> > Best Regards, -- Kiwamu Okabe at VAJ
