[PATCH 0/2] platform/x86: asus-wmi: Serialize WMI evaluations and drop redundant rfkill lock

[PATCH 0/2] platform/x86: asus-wmi: Serialize WMI evaluations and drop redundant rfkill lock

[Marco Scardovi]

ASUS WMI management methods share a single firmware/EC mailbox that is
not re-entrant. Concurrent evaluations from ACPI notify, sysfs, HID
callers (hid-asus, asus-armoury) and debugfs can trigger nested SMIs or
corrupt the mailbox.

This series serializes every ASUS_WMI_MGMT_GUID evaluation through a
global mutex, then drops the old per-device wmi_lock that only covered
WLAN rfkill hotplug.

The mutex is static because asus_wmi_evaluate_method() is exported and
callers outside asus-wmi have no struct asus_wmi. Fixes points at the
original shared evaluate helper rather than the later export, so stable
backports are not limited to kernels that carry that export.

Patch 1 introduces asus_wmi_evaluate_method_locked() and routes method3,
method5, method_buf and show_call through it.

Patch 2 removes the now-redundant wmi_lock and the pass-through
asus_rfkill_wlan_ops.

Link: https://github.com/OpenGamingCollective/asusctl/issues/328

Marco Scardovi (2):
  platform/x86: asus-wmi: Serialize WMI method evaluations with a mutex
  platform/x86: asus-wmi: Remove redundant per-device wmi_lock and
    duplicate rfkill ops

 drivers/platform/x86/asus-wmi.c | 67 +++++++++++----------------------
 1 file changed, 23 insertions(+), 44 deletions(-)

-- 
2.55.0

[PATCH 1/2] platform/x86: asus-wmi: Serialize WMI method evaluations with a mutex

[Marco Scardovi]

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.

The mutex is non-recursive: nested ACPI/WMI notify handlers must not call
back into evaluate on the same task (defer via workqueue, as
asus_rfkill_notify already does).

The sysfs-versus-notify race predates the later export of
asus_wmi_evaluate_method(), so Fixes points at the original shared
evaluate helper rather than that export.

Link: https://github.com/OpenGamingCollective/asusctl/issues/328
Fixes: d33da3b68669 ("asus-wmi: factorise wmi_evaluate_method call")
Cc: stable@vger.kernel.org
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
Reviewed-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/platform/x86/asus-wmi.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index a65090429ca7..9c69b9b24104 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/backlight.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/dmi.h>
@@ -28,6 +29,7 @@
 #include <linux/leds.h>
 #include <linux/minmax.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/pci_hotplug.h>
 #include <linux/platform_data/x86/asus-wmi.h>
@@ -353,6 +355,20 @@ static void asus_wmi_show_deprecated(void)
 
 /* WMI ************************************************************************/
 
+/*
+ * Concurrent evaluations of ASUS WMI methods can re-enter firmware
+ * SMI/EC mailbox handling and corrupt the mailbox.
+ */
+static DEFINE_MUTEX(asus_wmi_eval_lock);
+
+static acpi_status asus_wmi_evaluate_method_locked(u32 method_id,
+						   struct acpi_buffer *input,
+						   struct acpi_buffer *output)
+{
+	guard(mutex)(&asus_wmi_eval_lock);
+	return wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id, input, output);
+}
+
 static int asus_wmi_evaluate_method3(u32 method_id,
 		u32 arg0, u32 arg1, u32 arg2, u32 *retval)
 {
@@ -367,8 +383,7 @@ static int asus_wmi_evaluate_method3(u32 method_id,
 	union acpi_object *obj;
 	u32 tmp = 0;
 
-	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id,
-				     &input, &output);
+	status = asus_wmi_evaluate_method_locked(method_id, &input, &output);
 
 	pr_debug("%s called (0x%08x) with args: 0x%08x, 0x%08x, 0x%08x\n",
 		__func__, method_id, arg0, arg1, arg2);
@@ -419,8 +434,7 @@ static int asus_wmi_evaluate_method5(u32 method_id,
 	union acpi_object *obj;
 	u32 tmp = 0;
 
-	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id,
-				     &input, &output);
+	status = asus_wmi_evaluate_method_locked(method_id, &input, &output);
 
 	pr_debug("%s called (0x%08x) with args: 0x%08x, 0x%08x, 0x%08x, 0x%08x, 0x%08x\n",
 		__func__, method_id, arg0, arg1, arg2, arg3, arg4);
@@ -467,8 +481,7 @@ static int asus_wmi_evaluate_method_buf(u32 method_id,
 	union acpi_object *obj;
 	int err = 0;
 
-	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 0, method_id,
-				     &input, &output);
+	status = asus_wmi_evaluate_method_locked(method_id, &input, &output);
 
 	pr_debug("%s called (0x%08x) with args: 0x%08x, 0x%08x\n",
 		__func__, method_id, arg0, arg1);
@@ -5026,9 +5039,8 @@ static int show_call(struct seq_file *m, void *data)
 	union acpi_object *obj;
 	acpi_status status;
 
-	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID,
-				     0, asus->debug.method_id,
-				     &input, &output);
+	status = asus_wmi_evaluate_method_locked(asus->debug.method_id,
+						 &input, &output);
 
 	if (ACPI_FAILURE(status))
 		return -EIO;
-- 
2.55.0

[PATCH 2/2] platform/x86: asus-wmi: Remove redundant per-device wmi_lock and duplicate rfkill ops

[Marco Scardovi]

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>
Reviewed-by: Denis Benato <denis.benato@linux.dev>
---
 drivers/platform/x86/asus-wmi.c | 37 ++-------------------------------
 1 file changed, 2 insertions(+), 35 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 9c69b9b24104..9ceaea504e94 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -332,7 +332,6 @@ struct asus_wmi {
 
 	struct hotplug_slot hotplug_slot;
 	struct mutex hotplug_lock;
-	struct mutex wmi_lock;
 	struct workqueue_struct *hotplug_workqueue;
 	struct work_struct hotplug_work;
 
@@ -2245,9 +2244,7 @@ static void asus_rfkill_hotplug(struct asus_wmi *asus)
 	bool absent;
 	u32 l;
 
-	mutex_lock(&asus->wmi_lock);
 	blocked = asus_wlan_rfkill_blocked(asus);
-	mutex_unlock(&asus->wmi_lock);
 
 	mutex_lock(&asus->hotplug_lock);
 	pci_lock_rescan_remove();
@@ -2449,30 +2446,6 @@ static void asus_rfkill_query(struct rfkill *rfkill, void *data)
 	rfkill_set_sw_state(priv->rfkill, !result);
 }
 
-static int asus_rfkill_wlan_set(void *data, bool blocked)
-{
-	struct asus_rfkill *priv = data;
-	struct asus_wmi *asus = priv->asus;
-	int ret;
-
-	/*
-	 * This handler is enabled only if hotplug is enabled.
-	 * In this case, the asus_wmi_set_devstate() will
-	 * trigger a wmi notification and we need to wait
-	 * this call to finish before being able to call
-	 * any wmi method
-	 */
-	mutex_lock(&asus->wmi_lock);
-	ret = asus_rfkill_set(data, blocked);
-	mutex_unlock(&asus->wmi_lock);
-	return ret;
-}
-
-static const struct rfkill_ops asus_rfkill_wlan_ops = {
-	.set_block = asus_rfkill_wlan_set,
-	.query = asus_rfkill_query,
-};
-
 static const struct rfkill_ops asus_rfkill_ops = {
 	.set_block = asus_rfkill_set,
 	.query = asus_rfkill_query,
@@ -2491,13 +2464,8 @@ static int asus_new_rfkill(struct asus_wmi *asus,
 	arfkill->dev_id = dev_id;
 	arfkill->asus = asus;
 
-	if (dev_id == ASUS_WMI_DEVID_WLAN &&
-	    asus->driver->quirks->hotplug_wireless)
-		*rfkill = rfkill_alloc(name, &asus->platform_device->dev, type,
-				       &asus_rfkill_wlan_ops, arfkill);
-	else
-		*rfkill = rfkill_alloc(name, &asus->platform_device->dev, type,
-				       &asus_rfkill_ops, arfkill);
+	*rfkill = rfkill_alloc(name, &asus->platform_device->dev, type,
+			       &asus_rfkill_ops, arfkill);
 
 	if (!*rfkill)
 		return -EINVAL;
@@ -2571,7 +2539,6 @@ static int asus_wmi_rfkill_init(struct asus_wmi *asus)
 	int result = 0;
 
 	mutex_init(&asus->hotplug_lock);
-	mutex_init(&asus->wmi_lock);
 
 	result = asus_new_rfkill(asus, &asus->wlan, "asus-wlan",
 				 RFKILL_TYPE_WLAN, ASUS_WMI_DEVID_WLAN);
-- 
2.55.0