| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3042649.e9J7NaK4W3@rafael.j.wysocki>
Date: Mon, 22 Dec 2025 20:05:44 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Linux ACPI <linux-acpi@...r.kernel.org>,
Jonathan Cameron <jonathan.cameron@...wei.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux PCI <linux-pci@...r.kernel.org>, Bjorn Helgaas <helgaas@...nel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Hans de Goede <hansg@...nel.org>,
Mario Limonciello <mario.limonciello@....com>
Subject:
[PATCH v2.1 1/8] ACPI: bus: Fix handling of _OSC errors in acpi_run_osc()
From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
The handling of _OSC errors in acpi_run_osc() is inconsistent and
arguably not compliant with the _OSC definition (cf. Section 6.2.12 of
ACPI 6.6 [1]).
Namely, if OSC_QUERY_ENABLE is not set in the capabilities buffer and
any of the error bits are set in the _OSC return buffer, acpi_run_osc()
returns an error code and the _OSC return buffer is discarded. However,
in that case, depending on what error bits are set, the return buffer
may contain acknowledged bits for features that need to be controlled by
the kernel going forward.
If the OSC_INVALID_UUID_ERROR bit is set, the request could not be
processed at all and so in that particular case discarding the _OSC
return buffer and returning an error is the right thing to do regardless
of whether or not OSC_QUERY_ENABLE is set in the capabilities buffer.
If OSC_QUERY_ENABLE is set in the capabilities buffer and the
OSC_REQUEST_ERROR or OSC_INVALID_REVISION_ERROR bits are set in the
return buffer, acpi_run_osc() may return an error and discard the _OSC
return buffer because in that case the platform configuration does not
change. However, if any of them is set in the return buffer when
OSC_QUERY_ENABLE is not set in the capabilities buffer, the feature
mask in the _OSC return buffer still representes a set of acknowleded
features as per the _OSC definition:
The platform acknowledges the Capabilities Buffer by returning a
buffer of DWORDs of the same length. Set bits indicate acknowledgment
that OSPM may take control of the capability and cleared bits indicate
that the platform either does not support the capability or that OSPM
may not assume control.
which is not conditional on the error bits being clear, so in that case,
discarding the _OSC return buffer is questionable. There is also no
reason to return an error and discard the _OSC return buffer if the
OSC_CAPABILITIES_MASK_ERROR bit is set in it, but printing diagnostic
messages is appropriate when that happens with OSC_QUERY_ENABLE clear
in the capabilities buffer.
Adress this issue by making acpi_run_osc() follow the rules outlined
above.
Moreover, make acpi_run_osc() only take the defined _OSC error bits into
account when checking _OSC errors.
Link: https://uefi.org/specs/ACPI/6.6/06_Device_Configuration.html#osc-operating-system-capabilities [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
v2 -> v2.1:
* Print diagnostic messages for all error bits set in the _OSC return buffer
when OSC_INVALID_UUID_ERROR is set.
* Strengthen the changelog language related to printing diagnostic messages
when OSC_CAPABILITIES_MASK_ERROR is set in the _OSC return buffer (Jonathan).
v1 -> v2:
* In addition to making the behavior consistent, also make the code
follow the specification more closely in the cases when the "query
enable" bit is not set in _OSC the capabilities buffer an error bits
are set in the _OSC return buffer.
* Update the changelog accordingly.
---
drivers/acpi/bus.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 14 deletions(-)
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -194,14 +194,18 @@ static void acpi_print_osc_error(acpi_ha
pr_debug("\n");
}
+#define OSC_ERROR_MASK (OSC_REQUEST_ERROR | OSC_INVALID_UUID_ERROR | \
+ OSC_INVALID_REVISION_ERROR | \
+ OSC_CAPABILITIES_MASK_ERROR)
+
acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
{
+ u32 errors, *capbuf = context->cap.pointer;
acpi_status status;
struct acpi_object_list input;
union acpi_object in_params[4];
union acpi_object *out_obj;
guid_t guid;
- u32 errors;
struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
if (!context)
@@ -240,29 +244,49 @@ acpi_status acpi_run_osc(acpi_handle han
status = AE_TYPE;
goto out_kfree;
}
- /* Need to ignore the bit0 in result code */
- errors = *((u32 *)out_obj->buffer.pointer) & ~(1 << 0);
+ /* Only take defined error bits into account. */
+ errors = *((u32 *)out_obj->buffer.pointer) & OSC_ERROR_MASK;
+ /*
+ * If OSC_QUERY_ENABLE is set, ignore the "capabilities masked"
+ * bit because it merely means that some features have not been
+ * acknowledged which is not unexpected.
+ */
+ if (capbuf[OSC_QUERY_DWORD] & OSC_QUERY_ENABLE)
+ errors &= ~OSC_CAPABILITIES_MASK_ERROR;
+
if (errors) {
+ /*
+ * As a rule, fail only if OSC_QUERY_ENABLE is set because
+ * otherwise the acknowledged features need to be controlled.
+ */
+ bool fail = !!(capbuf[OSC_QUERY_DWORD] & OSC_QUERY_ENABLE);
+
+ if (errors & OSC_INVALID_UUID_ERROR) {
+ acpi_print_osc_error(handle, context,
+ "_OSC invalid UUID");
+ /*
+ * Always fail if this bit is set because it means that
+ * the request could not be processed.
+ */
+ fail = true;
+ goto out_kfree;
+ }
if (errors & OSC_REQUEST_ERROR)
acpi_print_osc_error(handle, context,
"_OSC request failed");
- if (errors & OSC_INVALID_UUID_ERROR)
- acpi_print_osc_error(handle, context,
- "_OSC invalid UUID");
if (errors & OSC_INVALID_REVISION_ERROR)
acpi_print_osc_error(handle, context,
"_OSC invalid revision");
- if (errors & OSC_CAPABILITIES_MASK_ERROR) {
- if (((u32 *)context->cap.pointer)[OSC_QUERY_DWORD]
- & OSC_QUERY_ENABLE)
- goto out_success;
- status = AE_SUPPORT;
+ if (errors & OSC_CAPABILITIES_MASK_ERROR)
+ acpi_print_osc_error(handle, context,
+ "_OSC capability bits masked");
+
+ if (fail) {
+ status = AE_ERROR;
goto out_kfree;
}
- status = AE_ERROR;
- goto out_kfree;
}
-out_success:
+
context->ret.length = out_obj->buffer.length;
context->ret.pointer = kmemdup(out_obj->buffer.pointer,
context->ret.length, GFP_KERNEL);
Powered by blists - more mailing lists