platform/x86: asus-wmi: Serialize WMI method evaluations and clean up rfkill locks by scardracs · Pull Request #12 · OpenGamingCollective/linux-unstable

scardracs · 2026-08-29T05:22:14Z

Summary of Changes

This series addresses firmware re-entrancy issues and EC mailbox buffer corruption caused by concurrent evaluations of ASUS WMI management methods, guarantees mutual exclusion across all callers (including external modules), and cleans up legacy redundant locking primitives in asus-wmi.

Following maintainer review feedback, this series has been streamlined:

Scoped strictly to drivers/platform/x86/asus-wmi.c (the HID patch has been decoupled to avoid cross-subsystem dependencies).
Avoids intrusive pointer-to-pointer refactoring or complex memory lifetime changes by using a clean, transparent asus_wmi_evaluate_method_locked() helper with guard(mutex).
Combines the rfkill lock and ops cleanup into a single, cohesive follow-up commit.

Commits in this Series

platform/x86: asus-wmi: Serialize WMI method evaluations with a mutex
- Introduces a centralized evaluation wrapper (asus_wmi_evaluate_method_locked()) protected by a global mutex (asus_wmi_eval_lock) using guard(mutex).
- Routes all evaluations of ASUS_WMI_MGMT_GUID (method3, method5, method_buf, and show_call) through this helper.
- A static mutex is used because asus_wmi_evaluate_method() is an exported symbol invoked by external modules (e.g. hid-asus, asus-armoury) without access to struct asus_wmi *drvdata, and the underlying ACPI/EC management method is a single shared hardware interface.
- Link: [Bug]: Rapid Fn+F3 (power-mode) presses cause hard system freeze on ROG Zephyrus G14 GA403UM asusctl#328
platform/x86: asus-wmi: Remove redundant per-device wmi_lock and duplicate rfkill ops
- With all WMI method evaluations serialized globally by asus_wmi_eval_lock, the per-device wmi_lock in struct asus_wmi is completely redundant.
- Removes wmi_lock from struct asus_wmi, asus_rfkill_hotplug(), and asus_wmi_rfkill_init().
- Drops the pass-through asus_rfkill_wlan_set() and asus_rfkill_wlan_ops, allocating WLAN rfkill devices with &asus_rfkill_ops directly.

Ghoul4500 · 2026-08-31T19:24:43Z

Don't link as closes like this. merging this PR should not close that issue, until the patchset makes it to mainline

NeroReflex · 2026-08-31T19:34:11Z

-static int asus_wmi_evaluate_method3(u32 method_id,
-		u32 arg0, u32 arg1, u32 arg2, u32 *retval)
+/* Serializes all evaluations of ASUS_WMI_MGMT_GUID methods */
+static DEFINE_MUTEX(asus_wmi_eval_lock);


Why is this mutex static? We have drvdata in asus-wmi

AFAIK asus_wmi_evaluate_method() is exported and used by standalone modules like hid-asus.ko and asus-armoury.ko, which don't have access to drvdata. Also, ASUS_WMI_MGMT_GUID is a global firmware interface, so we need one global mutex to serialize access across modules.

NeroReflex · 2026-08-31T19:34:40Z

-		.arg2 = arg2,
-	};
-	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
+	struct acpi_buffer input = { (acpi_size) sizeof(*args), args };


What happened here?

In asus_wmi_evaluate_method_raw(), args is passed in as a pointer (struct bios_args *args) rather than a local stack struct

NeroReflex · 2026-08-31T19:48:22Z

 	return 0;
 }

-int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval)


Changes to rfkill mixed with asus_wmi_evaluate_method. Not good. One patch should do as little as possible.

This goes for both comments: do you prefer a 4th patch or to remove it completely?

EDIT: I made it a 4th commit

NeroReflex · 2026-08-31T19:49:50Z

+		return -EIO;

+	*obj_ret = output.pointer;
 	return 0;


This thing returns either 0 or an error. It could be worth to return the ERR_PTR(err) or output.pointer, getting rid of the *obj altogether.

In his own patch where you convert all usages.

This one where tricky for me to answer so I asked gemini directly.

Returning ERR_PTR is an option, but with __free(kfree) used for RAII cleanup on obj, kfree() will crash if it receives an ERR_PTR on error paths (while it safely no-ops on NULL). Keeping the int return with *obj_ret = NULL on error keeps __free(kfree) leak-proof without needing custom error-pointer cleanup macros. Would you still prefer an ERR_PTR approach with a custom free handler?

NeroReflex · 2026-08-31T19:50:25Z

+	guard(mutex)(&asus_wmi_eval_lock);

-	kfree(obj);
+	pr_debug("%s called (0x%08x) with args: 0x%08x, 0x%08x, 0x%08x, 0x%08x, 0x%08x\n",


A print while locked?

Keeping it inside the lock ensures the debug log reflects the exact serialization order entering the BIOS. Do you see any specific issue with having pr_debug here?

NeroReflex · 2026-08-31T19:51:02Z

 		.arg3 = arg3,
 		.arg4 = arg4,
 	};
-	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };


Same as above: big changes together with rfkill.

NeroReflex · 2026-08-31T19:52:20Z

 		return -ENODEV;
 	}

 	return 0;


same as before: ERR_PTR() while you are at it. This thing, as the last one returns either 0 or an error, but (maybe) changes a pointer: let it return the pointer/pointer error.

NeroReflex · 2026-08-31T19:54:15Z

+	union acpi_object *obj __free(kfree) = NULL;
+	int err;

-	if (ACPI_FAILURE(status))


to me it doesn't look like ACPI_FAILURE is contained within asus_wmi_evaluate_method_raw (?)

It is handled right inside asus_wmi_evaluate_method_raw():

status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id, &input, &output); if (ACPI_FAILURE(status)) return -EIO;

So any ACPI evaluation failure is caught immediately and returns -EIO.

NeroReflex · 2026-08-31T19:57:37Z

-	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
-	acpi_status status;
-	union acpi_object *obj;
+	union acpi_object *obj __free(kfree) = NULL;


Not very good to see, the alternative is not that good either (result of the reworked method) because obj could end up storing an error. Maybe a macro?

For this one I asked gemini too because I did not have a good enough answer

Checking if (obj && obj->type == ACPI_TYPE_INTEGER) is standard ACPICA idiom across drivers/platform/x86/. Since _raw returns a negative error on failure and guarantees *obj_ret = NULL, obj never contains an error pointer. Would a custom macro be preferred here over the standard ACPI type check?

NeroReflex · 2026-08-31T20:01:39Z

 	}

-	if (retval != 0) {
+	if (retval > 1) {


Accepting a failure, reported properly as a failure, with a success is not acceptable.

The warning is correct here, at most you can specialize the error handling to print a custom message and/or use pr_warn_once.

Same goes for the above one.

Regardless: multi subsystem patches will follow time of both. Be ready to wait a long time.

On ASUS ACPI WMI, calling DEVS with ASUS_WMI_METHODID_NOTIF returns 1 for active/toggled state and 0 for disabled/off state. Both 0 and 1 are valid success states returned by the BIOS. Treating 1 as a failure (retval != 0) can cause spurious Failed to notify asus-wmi (retval): 0x1 warnings when pressing the fan/Aura key on ROG/TUF laptops. Only values > 1 (like ASUS_WMI_UNSUPPORTED_METHOD) are actual failures. Does that make sense, or did you have a different behavior in mind?

NeroReflex · 2026-08-31T20:04:10Z

-			__func__, method_id, arg0, -EIO);
-		return -EIO;
+			__func__, method_id, arg0, err);
+		return err;


You are changing the logic here. Either you change the logic or you refactor: not both in the same patch.

asus_wmi_evaluate_method_raw() returns -EIO on ACPI_FAILURE, so err evaluates to -EIO exactly as before. The logic is identical, it just avoids hardcoding -EIO in multiple places. Would you prefer keeping the explicit -EIO literal in the debug print instead?

NeroReflex · 2026-08-31T20:06:43Z

 	return 0;
 }

+static int asus_wmi_evaluate_method3(u32 method_id,


These got moved? why?

Because asus_wmi_evaluate_method3() now calls asus_wmi_evaluate_method5() (passing 0 for the unused args) to eliminate duplicate WMI call boilerplate. In C, it has to be defined below asus_wmi_evaluate_method5() to avoid needing a forward declaration.

scardracs · 2026-09-01T07:37:29Z

Don't link as closes like this. merging this PR should not close that issue, until the patchset makes it to mainline

Good point, thank. I'll change it to Link:

scardracs · 2026-09-03T06:42:15Z

@NeroReflex I've made the rebase and now it is in line with OGC's master

NeroReflex · 2026-09-03T07:41:23Z

@NeroReflex I've made the rebase and now it is in line with OGC's master

Very well. Now what's missing? Changing commits?

Concurrent evaluations of ASUS WMI management methods (from ACPI notify, HID, userspace daemons, and debugfs) enter the BIOS ACPI/SMM interface simultaneously, triggering re-entrant SMIs or EC mailbox buffer corruption. Fix this at the root by introducing a centralized evaluation helper (asus_wmi_evaluate_method_locked()) protected by a global mutex (asus_wmi_eval_lock) using guard(mutex). Route all evaluations of ASUS_WMI_MGMT_GUID (method3, method5, method_buf, and show_call) through this helper. A static mutex is required because asus_wmi_evaluate_method() is an exported symbol used by external modules (such as hid-asus and asus-armoury) that lack access to struct asus_wmi drvdata, and the underlying ASUS ACPI/EC management method is a single physical platform resource. Link: OpenGamingCollective/asusctl#328 Cc: stable@vger.kernel.org Signed-off-by: Marco Scardovi <scardracs@disroot.org>

…icate rfkill ops With all WMI method evaluations serialized globally by asus_wmi_eval_lock in asus_wmi_evaluate_method_locked(), the per-device wmi_lock in struct asus_wmi is completely redundant. Remove wmi_lock from struct asus_wmi, its initialization in asus_wmi_rfkill_init(), and its manual locking in asus_rfkill_hotplug(). Consequently, asus_rfkill_wlan_set() becomes a simple pass-through to asus_rfkill_set(), rendering asus_rfkill_wlan_ops identical to asus_rfkill_ops. Drop asus_rfkill_wlan_set() and asus_rfkill_wlan_ops, allocating WLAN rfkill devices with &asus_rfkill_ops directly. Signed-off-by: Marco Scardovi <scardracs@disroot.org>

scardracs changed the title ~~platform/x86: asus-wmi & HID: asus: Serialize WMI method evaluations and fix event return handling~~ platform/x86: asus-wmi & HID: asus: Serialize WMI evaluations, clean up wmi_lock, and handle non-zero success codes Aug 29, 2026

scardracs marked this pull request as draft August 29, 2026 09:18

scardracs marked this pull request as ready for review August 29, 2026 09:21

scardracs force-pushed the asus-fixes branch from b0abfa9 to 1d78af2 Compare August 29, 2026 09:24

scardracs changed the title ~~platform/x86: asus-wmi & HID: asus: Serialize WMI evaluations, clean up wmi_lock, and handle non-zero success codes~~ platform/x86: asus-wmi & HID: asus: Serialize WMI evaluations and handle non-zero success codes Aug 29, 2026

scardracs changed the title ~~platform/x86: asus-wmi & HID: asus: Serialize WMI evaluations and handle non-zero success codes~~ platform/x86: asus-wmi, HID: asus: Serialize WMI method evaluations and clean up locks Aug 30, 2026

scardracs force-pushed the asus-fixes branch from ed29d13 to 55216ad Compare August 30, 2026 14:16

NeroReflex reviewed Aug 31, 2026

View reviewed changes

NeroReflex force-pushed the master branch from d36eccf to 370d86f Compare September 1, 2026 19:43

scardracs force-pushed the asus-fixes branch from 55216ad to 4431a5d Compare September 3, 2026 06:41

scardracs force-pushed the asus-fixes branch from 4431a5d to a19e8a5 Compare September 4, 2026 07:59

scardracs changed the title ~~platform/x86: asus-wmi, HID: asus: Serialize WMI method evaluations and clean up locks~~ platform/x86: asus-wmi: Serialize WMI method evaluations and clean up rfkill locks Sep 4, 2026

github-actions Bot force-pushed the master branch from af0e02d to 743e77c Compare September 4, 2026 13:12

scardracs added 2 commits September 4, 2026 17:11

scardracs force-pushed the asus-fixes branch from a19e8a5 to 2a4ae2d Compare September 4, 2026 15:12

Uh oh!

Conversation

scardracs commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Commits in this Series

Uh oh!

Ghoul4500 commented Aug 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scardracs Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NeroReflex Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scardracs Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scardracs commented Sep 1, 2026

Uh oh!

scardracs commented Sep 3, 2026

Uh oh!

NeroReflex commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

scardracs commented Aug 29, 2026 •

edited

Loading

scardracs Sep 1, 2026 •

edited

Loading

NeroReflex Aug 31, 2026 •

edited

Loading

scardracs Sep 1, 2026 •

edited

Loading