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 <[email protected]>
---
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);
return_ACPI_STATUS(status);
}
--
2.17.1
On Wed, Oct 19, 2022 at 9:38 AM Chen Zhongjin <[email protected]> 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 <[email protected]>
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);
>
> return_ACPI_STATUS(status);
> }
> --
Bob, this looks correct to me, but I may be missing something in which
case please let me know.
On Fri, Oct 28, 2022 at 5:46 PM Rafael J. Wysocki <[email protected]> wrote:
>
> On Wed, Oct 19, 2022 at 9:38 AM Chen Zhongjin <[email protected]> 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 <[email protected]>
>
> 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.
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.
Hi,
On 2022/11/6 3:00, Rafael J. Wysocki wrote:
> On Fri, Oct 28, 2022 at 5:46 PM Rafael J. Wysocki <[email protected]> wrote:
>> On Wed, Oct 19, 2022 at 9:38 AM Chen Zhongjin <[email protected]> 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 <[email protected]>
>> 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.
On Mon, Nov 7, 2022 at 10:30 AM Chen Zhongjin <[email protected]> wrote:
>
> Hi,
>
> On 2022/11/6 3:00, Rafael J. Wysocki wrote:
> > On Fri, Oct 28, 2022 at 5:46 PM Rafael J. Wysocki <[email protected]> wrote:
> >> On Wed, Oct 19, 2022 at 9:38 AM Chen Zhongjin <[email protected]> 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 <[email protected]>
> >> 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"
At this point I have my own version of the fix, please see:
https://patchwork.kernel.org/project/linux-acpi/patch/2669303.mvXUDI8C0e@kreacher/