[PATCH v1] ACPICA: Refuse to evaluate a method if arguments are missing

Rafael J. Wysocki posted 1 patch 3 months, 3 weeks ago
drivers/acpi/acpica/dsmethod.c |    7 +++++++
1 file changed, 7 insertions(+)
[PATCH v1] ACPICA: Refuse to evaluate a method if arguments are missing
Posted by Rafael J. Wysocki 3 months, 3 weeks ago
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

As reported in [1], a platform firmware update that increased the number
of method parameters and forgot to update a least one of its callers,
caused ACPICA to crash due to use-after-free.

Since this a result of a clear AML issue that arguably cannot be fixed
up by the interpreter (it cannot produce missing data out of thin air),
address it by making ACPICA refuse to evaluate a method if the caller
attempts to pass fewer arguments than expected to it.

Closes: https://github.com/acpica/acpica/issues/1027 [1]
Reported-by: Peter Williams <peter@newton.cx>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is an exception as it fixes a kernel crash on multiple platforms
affected by the defective platform firmware update.

I will take care of submitting an equivalent change to upstream
ACPICA later.

---
 drivers/acpi/acpica/dsmethod.c |    7 +++++++
 1 file changed, 7 insertions(+)

--- a/drivers/acpi/acpica/dsmethod.c
+++ b/drivers/acpi/acpica/dsmethod.c
@@ -483,6 +483,13 @@
 		return_ACPI_STATUS(AE_NULL_OBJECT);
 	}
 
+	if (this_walk_state->num_operands < obj_desc->method.param_count) {
+		ACPI_ERROR((AE_INFO, "Missing argument for method [%4.4s]",
+			    acpi_ut_get_node_name(method_node)));
+
+		return_ACPI_STATUS(AE_AML_UNINITIALIZED_ARG);
+	}
+
 	/* Init for new method, possibly wait on method mutex */
 
 	status =
Re: [PATCH v1] ACPICA: Refuse to evaluate a method if arguments are missing
Posted by Hans de Goede 3 months, 3 weeks ago
Hi,

On 18-Jun-25 2:17 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> As reported in [1], a platform firmware update that increased the number
> of method parameters and forgot to update a least one of its callers,
> caused ACPICA to crash due to use-after-free.
> 
> Since this a result of a clear AML issue that arguably cannot be fixed
> up by the interpreter (it cannot produce missing data out of thin air),
> address it by making ACPICA refuse to evaluate a method if the caller
> attempts to pass fewer arguments than expected to it.
> 
> Closes: https://github.com/acpica/acpica/issues/1027 [1]
> Reported-by: Peter Williams <peter@newton.cx>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thanks, patch looks good to me and I've also tested it:

Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Hans de Goede <hansg@kernel.org> # Dell XPS 9640 with BIOS 1.12.0

Regards,

Hans





> ---
> 
> This is an exception as it fixes a kernel crash on multiple platforms
> affected by the defective platform firmware update.
> 
> I will take care of submitting an equivalent change to upstream
> ACPICA later.
> 
> ---
>  drivers/acpi/acpica/dsmethod.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> --- a/drivers/acpi/acpica/dsmethod.c
> +++ b/drivers/acpi/acpica/dsmethod.c
> @@ -483,6 +483,13 @@
>  		return_ACPI_STATUS(AE_NULL_OBJECT);
>  	}
>  
> +	if (this_walk_state->num_operands < obj_desc->method.param_count) {
> +		ACPI_ERROR((AE_INFO, "Missing argument for method [%4.4s]",
> +			    acpi_ut_get_node_name(method_node)));
> +
> +		return_ACPI_STATUS(AE_AML_UNINITIALIZED_ARG);
> +	}
> +
>  	/* Init for new method, possibly wait on method mutex */
>  
>  	status =
> 
> 
>