lkml.org 
[lkml]   [2022]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml()
From
Hi,

On 2022/11/6 3:00, Rafael J. Wysocki wrote:
> On Fri, Oct 28, 2022 at 5:46 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Wed, Oct 19, 2022 at 9:38 AM Chen Zhongjin <chenzhongjin@huawei.com> wrote:
>>> KASAN reports a use-after-free problem and causes kernel panic
>>> triggered by: modprobe acpiphp_ibm
>>>
>>> BUG: KASAN:
>>> use-after-free in acpi_ds_dump_method_stack (drivers/acpi/acpica/dsdebug.c:145)
>>> Read of size 8 at addr ffff888002f843f0 by task modprobe/519
>>>
>>> CPU: 2 PID: 519 Comm: modprobe Not tainted 6.0.0+
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
>>> Call Trace:
>>> <TASK>
>>> acpi_ds_dump_method_stack (drivers/acpi/acpica/dsdebug.c:145)
>>> acpi_ds_method_error (drivers/acpi/acpica/dsmethod.c:232)
>>> acpi_ps_parse_aml (drivers/acpi/acpica/psparse.c:607)
>>> ...
>>> </TASK>
>>>
>>> Allocated by task 519:
>>> ...
>>> __kasan_kmalloc (mm/kasan/common.c:526)
>>> acpi_ds_create_walk_state (drivers/acpi/acpica/dswstate.c:519)
>>> acpi_ds_call_control_method (drivers/acpi/acpica/dsmethod.c:498)
>>> acpi_ps_parse_aml (drivers/acpi/acpica/psparse.c:607)
>>> ...
>>>
>>> Freed by task 519:
>>> ...
>>> __kmem_cache_free+0xb6/0x3c0
>>> acpi_ds_delete_walk_state (drivers/acpi/acpica/dswstate.c:722)
>>> acpi_ds_call_control_method (drivers/acpi/acpica/dsmethod.c:586)
>>> acpi_ps_parse_aml (drivers/acpi/acpica/psparse.c:607)
>>> ...
>>> ---[ end Kernel panic - not syncing: Fatal exception ]---
>>>
>>> In the error path in acpi_ps_parse_aml():
>>>
>>> acpi_ds_call_control_method()
>>> acpi_ds_create_walk_state()
>>> acpi_ds_push_walk_state()
>>> # thread->walk_state_list = walk_state
>>>
>>> acpi_ds_init_aml_walk # *fail*
>>> goto cleanup:
>>> acpi_ds_delete_walk_state() # ACPI_FREE(walk_state)
>>>
>>> acpi_ds_method_error()
>>> acpi_ds_dump_method_stack()
>>> # using freed thread->walk_state_list
>>>
>>> Briefly, the walk_state is pushed to thread, and freed without being poped.
>>> Then it is used in acpi_ds_dump_method_stack() and causes use-after-free.
>>>
>>> Add acpi_ds_pop_walk_state(thread) to the error path to fix the problem.
>>>
>>> Fixes: 0bac4295526c ("ACPICA: Dispatcher: Move stack traversal code to dispatcher")
>>>
>>> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
>> This should be submitted to the upstream project on GitHub, but it
>> looks bad enough, so I'll take care of this.
>>
>> Applied as 6.1-rc material, thanks!
>>
>>> ---
>>> drivers/acpi/acpica/dsmethod.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/acpi/acpica/dsmethod.c b/drivers/acpi/acpica/dsmethod.c
>>> index ae2e768830bf..19da7fc73186 100644
>>> --- a/drivers/acpi/acpica/dsmethod.c
>>> +++ b/drivers/acpi/acpica/dsmethod.c
>>> @@ -581,6 +581,7 @@ acpi_ds_call_control_method(struct acpi_thread_state *thread,
>>>
>>> acpi_ds_terminate_control_method(obj_desc, next_walk_state);
>>> acpi_ds_delete_walk_state(next_walk_state);
>>> + acpi_ds_pop_walk_state(thread);
> On second thought, though, should it be popped before deleting?
> Otherwise it looks like there will be still use-after-free, because
> acpi_ds_pop_walk_state() accesses the walk_state at the top of the
> queue.

You are right it is wrong and sorry I didn't notice that.

I have reproduced same problem on current tree... Have no idea why I
missed it before.


I noticed that this patch have been on next-tree so I submitted another
one to fix it.

See "ACPICA: Fix pop_walk_state called after walk_state is deleted"


Thanks for your time!

Best,

Chen

> Moreover, it is not correct to pop the walk state if next_walk_state
> is NULL AFAICS.
>
> I'm dropping this one.
>
>
>>> return_ACPI_STATUS(status);
>>> }
>>> --
>> Bob, this looks correct to me, but I may be missing something in which
>> case please let me know.

\
 
 \ /
  Last update: 2022-11-07 10:31    [W:0.129 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site