[PATCH AUTOSEL 5.4 6/7] ACPICA: Refuse to evaluate a method if arguments are missing

[PATCH AUTOSEL 5.4 6/7] ACPICA: Refuse to evaluate a method if arguments are missing

[Sasha Levin]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

[ Upstream commit 6fcab2791543924d438e7fa49276d0998b0a069f ]

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>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Hans de Goede <hansg@kernel.org> # Dell XPS 9640 with BIOS 1.12.0
Link: https://patch.msgid.link/5909446.DvuYhMxLoT@rjwysocki.net
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

**YES**

This commit should be backported to stable kernel trees. Here's my
extensive analysis:

## Key Reasons for Backporting:

### 1. **Critical Security Bug - Use-After-Free**
The commit message explicitly states this fixes a use-after-free
vulnerability that causes ACPICA to crash. Use-after-free bugs are
severe security issues that can lead to:
- System crashes and denial of service
- Potential arbitrary code execution
- Memory corruption

### 2. **Real-World Impact**
The commit references an actual bug report with a specific
hardware/firmware scenario:
- Platform firmware update increased method parameters
- Firmware forgot to update at least one caller
- This caused a real crash on actual hardware (Dell XPS 9640 with BIOS
  1.12.0)
- The bug has been reported, tested, and verified by multiple people

### 3. **Small, Focused Fix**
The code change is minimal and surgical:
```c
+       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);
+       }
```
This adds a simple bounds check before method execution, which is
exactly the type of defensive programming that should be in stable
kernels.

### 4. **Prevents Memory Corruption**
Looking at the surrounding code (specifically line 542 in the original):
```c
for (i = 0; i < obj_desc->method.param_count; i++) {
    acpi_ut_remove_reference(this_walk_state->operands[i]);
    this_walk_state->operands[i] = NULL;
}
```
Without the check, if `num_operands < param_count`, this loop would
access beyond the valid operands, causing use-after-free.

### 5. **Clear Error Handling**
The fix properly returns `AE_AML_UNINITIALIZED_ARG`, which is an
existing ACPICA error code specifically designed for this scenario
("Method tried to use an uninitialized argument"). This maintains API
compatibility.

### 6. **No Architectural Changes**
The commit:
- Doesn't introduce new features
- Doesn't change existing behavior for valid code
- Only adds validation to prevent crashes from invalid AML
- Is confined to the ACPICA subsystem

### 7. **Firmware Bug Mitigation**
This is a defensive fix against firmware bugs, which is exactly the type
of robustness stable kernels need. The kernel should not crash due to
firmware mistakes.

## Comparison with Similar Commits:

Unlike the similar commits shown (which were mostly about improving
error messages or cosmetic changes), this commit:
- Fixes an actual crash/security issue
- Has been reported and tested on real hardware
- Prevents memory corruption
- Is not just a theoretical improvement

This aligns perfectly with stable kernel rules: important bug fixes with
minimal risk that improve system stability and security.

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

diff --git a/drivers/acpi/acpica/dsmethod.c b/drivers/acpi/acpica/dsmethod.c
index 603483f8332b0..203e9ee47fdb8 100644
--- a/drivers/acpi/acpica/dsmethod.c
+++ b/drivers/acpi/acpica/dsmethod.c
@@ -483,6 +483,13 @@ acpi_ds_call_control_method(struct acpi_thread_state *thread,
 		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 =
-- 
2.39.5