Re: [PATCH] ACPICA: Fix parse tree memory leak on AML execution error unwind

[Rafael J. Wysocki (Intel)]

On Thu, Oct 15, 2026 at 3:28 AM Iurii Trukhin <yuri@xxxxxxxxxxx> wrote:
>
> acpi_ps_complete_this_op() decides whether to defer deletion of an
> AML_CLASS_ARGUMENT parse object to its enclosing parent by checking
> walk_state->op_info->class instead of the opcode class of the op being
> completed.
>
> When acpi_ps_complete_op() or acpi_ps_complete_final_op() unwinds open
> parser scopes on AE_CTRL_TERMINATE or an execution error (such as
> AE_NOT_FOUND when resolving a missing symbol inside a control method),
> it pops each enclosing parent op and passes it to
> acpi_ps_complete_this_op() while walk_state->op_info still points to the
> last child op that failed. If that failing child op was of class
> AML_CLASS_ARGUMENT (for example, AML_INT_NAMEPATH_OP when
> acpi_ps_get_arguments() fails to resolve a method call or predicate
> operand), acpi_ps_complete_this_op() sees AML_CLASS_ARGUMENT for every
> popped parent op and returns AE_OK early without deleting any of the
> enclosing parent subtrees.
>
> For nested control methods invoked via acpi_ds_call_control_method(),
> the walk state is initialized with root_op = NULL, so top-level ops in
> the called method (such as AML_IF_OP) have op->common.parent == NULL
> and are never attached to a root scope op. Skipping
> acpi_ps_delete_parse_tree() during error unwind leaks the entire parse
> subtree (both Acpi-ParseExt and Acpi-Parse objects, the latter merged
> by SLUB into the 80-byte Acpi-State kmem_cache on 64-bit kernels) on
> every failed GPE or method evaluation.
>
> Look up the opcode info of op->common.aml_opcode directly in
> acpi_ps_complete_this_op() and only defer deletion of
> AML_CLASS_ARGUMENT ops when op->common.parent is non-NULL, without
> mutating walk_state->opcode during error unwind (which acpi_ps_parse_loop()
> still inspects when skipping failed module-level scopes). Remove the
> now-redundant acpi_ps_delete_parse_tree(*op) inside the AML_REGION_OP /
> AML_DATA_REGION_OP error block in acpi_ps_complete_op(), which previously
> worked around this bug only for region ops and caused a double-free when
> region creation failed on a non-argument child op. Also use
> acpi_ut_delete_generic_state() instead of ACPI_FREE() when draining
> walk_state->control_state in acpi_ps_parse_loop().
>
> Fixes: 5088814a6e93 ("ACPICA: AML parser: attempt to continue loading
> table after error")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221241
> Signed-off-by: Iurii Trukhin <yuri@xxxxxxxxxxx>

Please submit this change as a pull request to the upstream ACPICA
project on GitHub.  The kernel will pick it up from there as per

Documentation/driver-api/acpi/linuxized-acpica.rst

> ---
>  drivers/acpi/acpica/psloop.c   | 5 +++--
>  drivers/acpi/acpica/psobject.c | 1 -
>  drivers/acpi/acpica/psparse.c  | 7 +++++--
>  3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/acpica/psloop.c b/drivers/acpi/acpica/psloop.c
> index 24a57f971c96..0b672a531907 100644
> --- a/drivers/acpi/acpica/psloop.c
> +++ b/drivers/acpi/acpica/psloop.c
> @@ -464,8 +464,9 @@ acpi_status acpi_ps_parse_loop(struct
> acpi_walk_state *walk_state)
>                                                 walk_state->aml =
>                                                     parser_state->aml;
>                                         }
> -                                       ACPI_FREE(acpi_ut_pop_generic_state
> -                                                 (&walk_state->control_state));
> +                                       acpi_ut_delete_generic_state
> +                                           (acpi_ut_pop_generic_state
> +                                            (&walk_state->control_state));
>                                 }
>                                 op = NULL;
>                                 continue;
> diff --git a/drivers/acpi/acpica/psobject.c b/drivers/acpi/acpica/psobject.c
> index 629cafab1930..f1425b98087b 100644
> --- a/drivers/acpi/acpica/psobject.c
> +++ b/drivers/acpi/acpica/psobject.c
> @@ -559,7 +559,6 @@ acpi_ps_complete_op(struct acpi_walk_state *walk_state,
>                                                                 node);
>                                         acpi_ns_remove_node((*op)->common.node);
>                                         (*op)->common.node = NULL;
> -                                       acpi_ps_delete_parse_tree(*op);
>                                 }
>
>                                 status2 =
> diff --git a/drivers/acpi/acpica/psparse.c b/drivers/acpi/acpica/psparse.c
> index 42ec8abef626..b5110d43ab7a 100644
> --- a/drivers/acpi/acpica/psparse.c
> +++ b/drivers/acpi/acpica/psparse.c
> @@ -108,6 +108,7 @@ acpi_ps_complete_this_op(struct acpi_walk_state *walk_state,
>  {
>         union acpi_parse_object *prev;
>         union acpi_parse_object *next;
> +       const struct acpi_opcode_info *op_info;
>         const struct acpi_opcode_info *parent_info;
>         union acpi_parse_object *replacement_op = NULL;
>         acpi_status status = AE_OK;
> @@ -124,9 +125,11 @@ acpi_ps_complete_this_op(struct acpi_walk_state
> *walk_state,
>
>         /* Delete this op and the subtree below it if asked to */
>
> +       op_info = acpi_ps_get_opcode_info(op->common.aml_opcode);
>         if (((walk_state->parse_flags & ACPI_PARSE_TREE_MASK) !=
> -            ACPI_PARSE_DELETE_TREE)
> -           || (walk_state->op_info->class == AML_CLASS_ARGUMENT)) {
> +            ACPI_PARSE_DELETE_TREE) ||
> +           ((op_info->class == AML_CLASS_ARGUMENT) &&
> +            op->common.parent)) {
>                 return_ACPI_STATUS(AE_OK);
>         }
>
> --
> 2.56.0
>