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

From: Armin Wolf
Date: Sat Feb 25 2023 - 06:52:10 EST


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@xxxxxx>
---
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