[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f2db2739-a11f-2b83-6859-584e279a8a52@valinux.co.jp>
Date: Thu, 27 Jul 2023 06:51:02 +0900
From: Kiwamu Okabe <okabe@...inux.co.jp>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ACPI: tables: Fix NULL dereference by
acpi_os_map_memory()
Dear Rafael,
On 7/26/23 23:35, Rafael J. Wysocki wrote:
> On Wed, Jul 26, 2023 at 6:53 AM Kiwamu Okabe <okabe@...inux.co.jp> 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@...inux.co.jp>
>> ---
>> 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
Powered by blists - more mailing lists