* [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml() @ 2022-10-19 7:34 Chen Zhongjin 2022-10-28 15:46 ` Rafael J. Wysocki 0 siblings, 1 reply; 5+ messages in thread From: Chen Zhongjin @ 2022-10-19 7:34 UTC (permalink / raw) To: linux-kernel, linux-acpi, devel Cc: robert.moore, rafael.j.wysocki, lenb, lv.zheng, chenzhongjin 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> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml() 2022-10-19 7:34 [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml() Chen Zhongjin @ 2022-10-28 15:46 ` Rafael J. Wysocki 2022-11-05 19:00 ` Rafael J. Wysocki 0 siblings, 1 reply; 5+ messages in thread From: Rafael J. Wysocki @ 2022-10-28 15:46 UTC (permalink / raw) To: Chen Zhongjin, robert.moore Cc: linux-kernel, linux-acpi, devel, rafael.j.wysocki, lenb, lv.zheng 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); > > return_ACPI_STATUS(status); > } > -- Bob, this looks correct to me, but I may be missing something in which case please let me know. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml() 2022-10-28 15:46 ` Rafael J. Wysocki @ 2022-11-05 19:00 ` Rafael J. Wysocki 2022-11-07 9:30 ` Chen Zhongjin 0 siblings, 1 reply; 5+ messages in thread From: Rafael J. Wysocki @ 2022-11-05 19:00 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Chen Zhongjin, robert.moore, linux-kernel, linux-acpi, rafael.j.wysocki, lenb, lv.zheng 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. 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml() 2022-11-05 19:00 ` Rafael J. Wysocki @ 2022-11-07 9:30 ` Chen Zhongjin 2022-11-07 17:51 ` Rafael J. Wysocki 0 siblings, 1 reply; 5+ messages in thread From: Chen Zhongjin @ 2022-11-07 9:30 UTC (permalink / raw) To: Rafael J. Wysocki Cc: robert.moore, linux-kernel, linux-acpi, rafael.j.wysocki, lenb, lv.zheng 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml() 2022-11-07 9:30 ` Chen Zhongjin @ 2022-11-07 17:51 ` Rafael J. Wysocki 0 siblings, 0 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2022-11-07 17:51 UTC (permalink / raw) To: Chen Zhongjin Cc: Rafael J. Wysocki, robert.moore, linux-kernel, linux-acpi, rafael.j.wysocki, lenb, lv.zheng On Mon, Nov 7, 2022 at 10:30 AM Chen Zhongjin <chenzhongjin@huawei.com> wrote: > > 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" At this point I have my own version of the fix, please see: https://patchwork.kernel.org/project/linux-acpi/patch/2669303.mvXUDI8C0e@kreacher/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-07 17:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-19 7:34 [PATCH] ACPICA: Fix use-after-free in acpi_ps_parse_aml() Chen Zhongjin 2022-10-28 15:46 ` Rafael J. Wysocki 2022-11-05 19:00 ` Rafael J. Wysocki 2022-11-07 9:30 ` Chen Zhongjin 2022-11-07 17:51 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).