⬢ DragonFlyBSD Kernel Audit
DF-5582 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/smbus/smbacpi/smbacpi.c
+++ b/sys/bus/smbus/smbacpi/smbacpi.c
@@ -37,6 +37,7 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
+#include <machine/atomic.h>
 #include <sys/module.h>
 #include <sys/errno.h>
 #include <sys/lock.h>
@@ -60,8 +61,12 @@
 struct acpi_i2c_handler_data {
 	struct acpi_connection_info info;
 	device_t dev;
+	int	busy;			/* in-flight space-handler count */
+	int	detaching;
 };
 
+#define	SMBACPI_DRAIN_TICKS	3000	/* 30 s cap; SMBus transfers are ms-scale */
+
 struct smbus_acpi_softc {
 	device_t dev;
 	device_t parent;
@@ -93,6 +98,8 @@
 	ACPI_STATUS s;
 
 	handle = acpi_get_handle(sc->parent);
+	sc->space_handler_data.busy = 0;
+	sc->space_handler_data.detaching = 0;
 	sc->space_handler_data.dev = sc->parent;
 	s = AcpiInstallAddressSpaceHandler(handle, ACPI_ADR_SPACE_GSBUS,
 	    &smbus_acpi_space_handler, NULL, &sc->space_handler_data);
@@ -140,10 +147,31 @@
 	if (Value == NULL)
 		return (AE_BAD_PARAMETER);
 
+	/*
+	 * Refuse dispatches once detach has started and account for
+	 * in-flight ones: AcpiRemoveAddressSpaceHandler() does NOT wait
+	 * for running handlers (they execute with the AML interpreter
+	 * exited, see AcpiEvAddressSpaceDispatch), and detach frees the
+	 * handler object -- including the ContextMutex the in-flight
+	 * dispatch still holds -- plus this softc (the handler Context)
+	 * and the parent ig4iic device underneath the running handler
+	 * (DF-5582).
+	 */
+	if (data->detaching)
+		return (AE_ABORT_METHOD);
+	atomic_add_int(&data->busy, 1);
+	if (data->detaching) {
+		atomic_subtract_int(&data->busy, 1);
+		return (AE_ABORT_METHOD);
+	}
+
 	s = AcpiBufferToResource(info->Connection, info->Length,
 	    &Resource);
-	if (ACPI_FAILURE(s))
+	if (ACPI_FAILURE(s)) {
+		atomic_subtract_int(&data->busy, 1);
+		wakeup(&data->busy);
 		return s;
+	}
 	if (Resource->Type != ACPI_RESOURCE_TYPE_SERIAL_BUS) {
 		s = AE_BAD_PARAMETER;
 		goto err;
@@ -237,6 +265,8 @@
 
 err:
 	ACPI_FREE(Resource);
+	atomic_subtract_int(&data->busy, 1);
+	wakeup(&data->busy);
 
 	return (s);
 }
@@ -269,7 +299,21 @@
 smbus_acpi_detach(device_t dev)
 {
 	struct smbus_acpi_softc *sc = device_get_softc(dev);
+	int t;
 
+	/*
+	 * Stop new GSBUS dispatches and drain in-flight ones BEFORE the
+	 * handler is unregistered: AcpiRemoveAddressSpaceHandler() frees
+	 * the handler object -- including the ContextMutex each dispatch
+	 * holds across the handler call -- and as soon as detach returns
+	 * this softc (the handler's Context) and the parent ig4iic device
+	 * are freed too (DF-5582).  Dispatches that start after the flag
+	 * see detaching and abort without touching hardware.
+	 */
+	sc->space_handler_data.detaching = 1;
+	for (t = 0; sc->space_handler_data.busy != 0 &&
+	    t < SMBACPI_DRAIN_TICKS; t++)
+		tsleep(&sc->space_handler_data.busy, 0, "smbdrn", 1);
 	smbus_acpi_remove_address_space_handler(sc);
 
 	return (0);