linux-kernel - [PATCH v2 2/4] ACPI: sbshc: Use ec query notifier call chain

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 25 Feb 2023 12:51:42 +0100
From:   Armin Wolf <W_Armin@....de>
To:     rafael@...nel.org, lenb@...nel.org
Cc:     linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v2 2/4] ACPI: sbshc: Use ec query notifier call chain

When using acpi_ec_add_query_handler(), a kernel oops
can occur when unloading the sbshc module, since the
handler callback might still be used by a work item
inside the ec workqueue.
Use the new ec query notifier call chain to register
the handler in a safe way. Return NOTIFY_BAD to override
the existing _Qxx handler in case the query was meant
for the EC SMBus controller.

Tested on a Acer Travelmate 4002WLMi.

Signed-off-by: Armin Wolf <W_Armin@....de>
---
 drivers/acpi/sbshc.c | 45 ++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index 16f2daaa2c45..e3280f646eb5 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -8,11 +8,14 @@
 #define pr_fmt(fmt) "ACPI: " fmt

 #include <linux/acpi.h>
+#include <linux/notifier.h>
 #include <linux/wait.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+
+#include "internal.h"
 #include "sbshc.h"

 #define ACPI_SMB_HC_CLASS	"smbus_host_ctl"
@@ -20,6 +23,7 @@

 struct acpi_smb_hc {
 	struct acpi_ec *ec;
+	struct notifier_block nb;
 	struct mutex lock;
 	wait_queue_head_t wait;
 	u8 offset;
@@ -194,6 +198,7 @@ int acpi_smbus_unregister_callback(struct acpi_smb_hc *hc)
 	hc->context = NULL;
 	mutex_unlock(&hc->lock);
 	acpi_os_wait_events_complete();
+
 	return 0;
 }

@@ -206,20 +211,28 @@ static inline void acpi_smbus_callback(void *context)
 		hc->callback(hc->context);
 }

-static int smbus_alarm(void *context)
+static int acpi_smbus_hc_notify(struct notifier_block *block, unsigned long action, void *data)
 {
-	struct acpi_smb_hc *hc = context;
+	struct acpi_smb_hc *hc = container_of(block, struct acpi_smb_hc, nb);
 	union acpi_smb_status status;
+	struct acpi_ec *ec = data;
 	u8 address;
+
+	if (ec != hc->ec || action != hc->query_bit)
+		return NOTIFY_DONE;
+
 	if (smb_hc_read(hc, ACPI_SMB_STATUS, &status.raw))
-		return 0;
+		return NOTIFY_OK;
+
 	/* Check if it is only a completion notify */
 	if (status.fields.done && status.fields.status == SMBUS_OK) {
 		hc->done = true;
 		wake_up(&hc->wait);
 	}
+
 	if (!status.fields.alarm)
-		return 0;
+		return NOTIFY_BAD;
+
 	mutex_lock(&hc->lock);
 	smb_hc_read(hc, ACPI_SMB_ALARM_ADDRESS, &address);
 	status.fields.alarm = 0;
@@ -233,20 +246,16 @@ static int smbus_alarm(void *context)
 					acpi_smbus_callback, hc);
 	}
 	mutex_unlock(&hc->lock);
-	return 0;
-}

-typedef int (*acpi_ec_query_func) (void *data);
-
-extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
-			      acpi_handle handle, acpi_ec_query_func func,
-			      void *data);
+	/* We may need to override existing _Qxx handlers */
+	return NOTIFY_BAD;
+}

 static int acpi_smbus_hc_add(struct acpi_device *device)
 {
-	int status;
 	unsigned long long val;
 	struct acpi_smb_hc *hc;
+	int status, ret;

 	if (!device)
 		return -EINVAL;
@@ -271,15 +280,19 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
 	hc->query_bit = val & 0xff;
 	device->driver_data = hc;

-	acpi_ec_add_query_handler(hc->ec, hc->query_bit, NULL, smbus_alarm, hc);
+	hc->nb.notifier_call = acpi_smbus_hc_notify;
+	ret = register_acpi_ec_query_notifier(&hc->nb);
+	if (ret < 0) {
+		kfree(hc);
+		return ret;
+	}
+
 	dev_info(&device->dev, "SBS HC: offset = 0x%0x, query_bit = 0x%0x\n",
 		 hc->offset, hc->query_bit);

 	return 0;
 }

-extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
-
 static void acpi_smbus_hc_remove(struct acpi_device *device)
 {
 	struct acpi_smb_hc *hc;
@@ -288,7 +301,7 @@ static void acpi_smbus_hc_remove(struct acpi_device *device)
 		return;

 	hc = acpi_driver_data(device);
-	acpi_ec_remove_query_handler(hc->ec, hc->query_bit);
+	unregister_acpi_ec_query_notifier(&hc->nb);
 	acpi_os_wait_events_complete();
 	kfree(hc);
 	device->driver_data = NULL;
--
2.30.2