| Message ID | 20240403004718.11902-1-W_Armin@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | ACPICA: Fix memory leak then namespace lookup fails | expand |
On Wed, Apr 3, 2024 at 2:47 AM Armin Wolf <W_Armin@gmx.de> wrote: > > When acpi_ps_get_next_namepath() fails due to a namespace lookup > failure, the acpi_parse_object is not freed before returning the > error code, causing a memory leak. > > Fix this by freeing the acpi_parse_object when encountering an > error. > > Tested-by: Dmitry Antipov <dmantipov@yandex.ru> > Signed-off-by: Armin Wolf <W_Armin@gmx.de> Because ACPICA is an external project supplying code to the Linux kernel, the way to change the ACPICA code in the kernel is to submit a pull request to the upstream ACPICA project on GitHub and once that PR has been merged, submit a Linux patch corresponding to it including the Link: tag pointing to the PR in question and the git ID of the corresponding upstream ACPICA commit. However, note that upstream ACPICA commits are automatically included into the Linux kernel source code every time the upstream ACPICA project makes a release, so it is not necessary to send the corresponding Linux patches for them unless in the cases when timing matters. > --- > drivers/acpi/acpica/psargs.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/acpi/acpica/psargs.c b/drivers/acpi/acpica/psargs.c > index 422c074ed289..7debfd5ce0d8 100644 > --- a/drivers/acpi/acpica/psargs.c > +++ b/drivers/acpi/acpica/psargs.c > @@ -820,6 +820,10 @@ acpi_ps_get_next_arg(struct acpi_walk_state *walk_state, > acpi_ps_get_next_namepath(walk_state, parser_state, > arg, > ACPI_NOT_METHOD_CALL); > + if (ACPI_FAILURE(status)) { > + acpi_ps_free_op(arg); > + return_ACPI_STATUS(status); > + } > } else { > /* Single complex argument, nothing returned */ > > @@ -854,6 +858,10 @@ acpi_ps_get_next_arg(struct acpi_walk_state *walk_state, > acpi_ps_get_next_namepath(walk_state, parser_state, > arg, > ACPI_POSSIBLE_METHOD_CALL); > + if (ACPI_FAILURE(status)) { > + acpi_ps_free_op(arg); > + return_ACPI_STATUS(status); > + } > > if (arg->common.aml_opcode == AML_INT_METHODCALL_OP) { > > -- > 2.39.2 > >
Am 08.04.24 um 16:29 schrieb Rafael J. Wysocki: > On Wed, Apr 3, 2024 at 2:47 AM Armin Wolf <W_Armin@gmx.de> wrote: >> When acpi_ps_get_next_namepath() fails due to a namespace lookup >> failure, the acpi_parse_object is not freed before returning the >> error code, causing a memory leak. >> >> Fix this by freeing the acpi_parse_object when encountering an >> error. >> >> Tested-by: Dmitry Antipov <dmantipov@yandex.ru> >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> > Because ACPICA is an external project supplying code to the Linux > kernel, the way to change the ACPICA code in the kernel is to submit a > pull request to the upstream ACPICA project on GitHub and once that PR > has been merged, submit a Linux patch corresponding to it including > the Link: tag pointing to the PR in question and the git ID of the > corresponding upstream ACPICA commit. > > However, note that upstream ACPICA commits are automatically included > into the Linux kernel source code every time the upstream ACPICA > project makes a release, so it is not necessary to send the > corresponding Linux patches for them unless in the cases when timing > matters. I submitted a PR to upstream ACPICA and the changes where accepted. Dmitry, do you think that this memory leak is critical? If not, then i think we can wait till the next ACPICA release. Thanks, Armin Wolf >> --- >> drivers/acpi/acpica/psargs.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/acpi/acpica/psargs.c b/drivers/acpi/acpica/psargs.c >> index 422c074ed289..7debfd5ce0d8 100644 >> --- a/drivers/acpi/acpica/psargs.c >> +++ b/drivers/acpi/acpica/psargs.c >> @@ -820,6 +820,10 @@ acpi_ps_get_next_arg(struct acpi_walk_state *walk_state, >> acpi_ps_get_next_namepath(walk_state, parser_state, >> arg, >> ACPI_NOT_METHOD_CALL); >> + if (ACPI_FAILURE(status)) { >> + acpi_ps_free_op(arg); >> + return_ACPI_STATUS(status); >> + } >> } else { >> /* Single complex argument, nothing returned */ >> >> @@ -854,6 +858,10 @@ acpi_ps_get_next_arg(struct acpi_walk_state *walk_state, >> acpi_ps_get_next_namepath(walk_state, parser_state, >> arg, >> ACPI_POSSIBLE_METHOD_CALL); >> + if (ACPI_FAILURE(status)) { >> + acpi_ps_free_op(arg); >> + return_ACPI_STATUS(status); >> + } >> >> if (arg->common.aml_opcode == AML_INT_METHODCALL_OP) { >> >> -- >> 2.39.2 >> >>
On 4/12/24 19:16, Armin Wolf wrote: > Dmitry, do you think that this memory leak is critical? If not, then i think > we can wait till the next ACPICA release. Hopefully not too critical. The leak itself is small, and it's highly depended from the particular notebook model and/or UEFI BIOS revision, so it should be safe to wait. Dmitry
diff --git a/drivers/acpi/acpica/psargs.c b/drivers/acpi/acpica/psargs.c index 422c074ed289..7debfd5ce0d8 100644 --- a/drivers/acpi/acpica/psargs.c +++ b/drivers/acpi/acpica/psargs.c @@ -820,6 +820,10 @@ acpi_ps_get_next_arg(struct acpi_walk_state *walk_state, acpi_ps_get_next_namepath(walk_state, parser_state, arg, ACPI_NOT_METHOD_CALL); + if (ACPI_FAILURE(status)) { + acpi_ps_free_op(arg); + return_ACPI_STATUS(status); + } } else { /* Single complex argument, nothing returned */ @@ -854,6 +858,10 @@ acpi_ps_get_next_arg(struct acpi_walk_state *walk_state, acpi_ps_get_next_namepath(walk_state, parser_state, arg, ACPI_POSSIBLE_METHOD_CALL); + if (ACPI_FAILURE(status)) { + acpi_ps_free_op(arg); + return_ACPI_STATUS(status); + } if (arg->common.aml_opcode == AML_INT_METHODCALL_OP) {