[PATCH v3 0/3] Input: applespi - fixes for DMA timeout, UAF, and NULL pointer dereference

[PATCH v3 0/3] Input: applespi - fixes for DMA timeout, UAF, and NULL pointer dereference

[Shih-Yuan Lee]

Hi Dmitry and the linux-input community,

This patch series addresses a long-standing DMA initialization timeout on the 
early 2015 12" MacBook (MacBook8,1) on any boot (including cold boot), as well 
as two pre-existing, high-severity UAF/NULL-pointer bugs in the applespi driver.

Changes in v3:
  - Added a `force_pio` module parameter to applespi to allow users to manually 
    disable DMA for SPI transfers.
  - Resolved the execute-after-free vulnerability by unconditionally restoring 
    the original can_dma callback (even if NULL) in the driver remove path.
  - Fixed the probe timing issue by applying the can_dma override at the very 
    beginning of applespi_probe() so that all early initialization transfers 
    safely use PIO mode, and properly restoring it in all probe error paths.
  - Documented the Bugzilla link in the commit message of Patch 1.
  - Format the `force_pio` description style to match other parameters.

Changes in v2:
  - Fixed an unbind/remove execute-after-free vulnerability by storing and 
    restoring the host controller's original can_dma callback in applespi_probe() 
    and applespi_remove().
  - Split the fixes into a 3-patch logical series.

Patch 1 introduces a DMI quirk and a `force_pio` module parameter in applespi.c 
to disable DMA. It overrides the shared controller's can_dma callback at the 
very beginning of applespi_probe() and restores it in all failure and removal 
paths.

Patch 2 fixes a pre-existing UAF vulnerability in the driver unbind path by 
explicitly calling cancel_work_sync() on the asynchronous registration worker 
work struct before devres frees the driver private data.

Patch 3 fixes a pre-existing race condition in the debugfs interface where 
userspace could open the tp_dim file before the asynchronous worker has 
finished initializing applespi->touchpad_input_dev, leading to a NULL 
pointer dereference. We resolve this using smp_load_acquire() and checking 
for NULL.

Best regards,
Shih-Yuan Lee

Shih-Yuan Lee (3):
  Input: applespi - force PIO mode on MacBook8,1
  Input: applespi - cancel pending work on driver remove
  Input: applespi - fix NULL pointer dereference in tp_dim open

 drivers/input/keyboard/applespi.c | 76 +++++++++++++++++++++++++------
 1 file changed, 63 insertions(+), 13 deletions(-)

-- 
2.39.5

[PATCH v3 1/3] Input: applespi - force PIO mode on MacBook8,1

[Shih-Yuan Lee]

On MacBook8,1 (early 2015 12" MacBook), the LPSS SPI controller's DMA
handshake and interrupt routing frequently fail or time out on boot,
causing the keyboard and trackpad to become unresponsive.

Address this by introducing a DMI quirk and a `force_pio` module
parameter. If either is true, override the SPI controller's `can_dma`
callback to a helper that always returns false. This forces all SPI
transfers to fall back to the reliable PIO mode.

To ensure correct timing and architectural safety:
1. Perform the override at the very beginning of applespi_probe() so
   that all early initialization transfers use PIO.
2. In case of probe failures or module unload (remove), unconditionally
   restore the original `can_dma` callback (even if it was NULL) to
   prevent any execute-after-free or dangling pointer vulnerability in
   the shared host controller.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/input/keyboard/applespi.c | 66 +++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index b5ff71cd5a70..79e5cb5001c7 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -46,6 +46,7 @@
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/efi.h>
+#include <linux/dmi.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
 #include <linux/ktime.h>
@@ -110,6 +111,10 @@ module_param_string(touchpad_dimensions, touchpad_dimensions,
 		    sizeof(touchpad_dimensions), 0444);
 MODULE_PARM_DESC(touchpad_dimensions, "The pixel dimensions of the touchpad, as XxY+W+H .");
 
+static bool applespi_force_pio;
+module_param_named(force_pio, applespi_force_pio, bool, 0444);
+MODULE_PARM_DESC(force_pio, "Force PIO mode (disables DMA) for SPI transfers. ([0] = disabled, 1 = enabled)");
+
 /**
  * struct keyboard_protocol - keyboard message.
  * message.type = 0x0110, message.length = 0x000a
@@ -431,6 +436,10 @@ struct applespi_data {
 	int				tp_dim_max_x;
 	int				tp_dim_min_y;
 	int				tp_dim_max_y;
+
+	bool				(*original_can_dma)(struct spi_controller *controller,
+							    struct spi_device *spi,
+							    struct spi_transfer *xfer);
 };
 
 static const unsigned char applespi_scancodes[] = {
@@ -1605,6 +1614,13 @@ static void applespi_save_bl_level(struct applespi_data *applespi,
 			 "Error saving backlight level to EFI vars: 0x%lx\n", sts);
 }
 
+static bool applespi_can_not_dma(struct spi_controller *controller,
+				 struct spi_device *spi,
+				 struct spi_transfer *xfer)
+{
+	return false;
+}
+
 static int applespi_probe(struct spi_device *spi)
 {
 	struct applespi_data *applespi;
@@ -1612,6 +1628,7 @@ static int applespi_probe(struct spi_device *spi)
 	acpi_status acpi_sts;
 	int sts, i;
 	unsigned long long gpe, usb_status;
+	bool override_dma = applespi_force_pio || dmi_match(DMI_PRODUCT_NAME, "MacBook8,1");
 
 	/* check if the USB interface is present and enabled already */
 	acpi_sts = acpi_evaluate_integer(spi_handle, "UIST", NULL, &usb_status);
@@ -1628,6 +1645,13 @@ static int applespi_probe(struct spi_device *spi)
 
 	applespi->spi = spi;
 
+	/* Save original can_dma and override if requested or needed */
+	applespi->original_can_dma = spi->controller->can_dma;
+	if (override_dma) {
+		dev_info(&spi->dev, "Disabling DMA for SPI to force PIO mode\n");
+		spi->controller->can_dma = applespi_can_not_dma;
+	}
+
 	INIT_WORK(&applespi->work, applespi_worker);
 
 	/* store the driver data */
@@ -1645,8 +1669,10 @@ static int applespi_probe(struct spi_device *spi)
 					       GFP_KERNEL);
 
 	if (!applespi->tx_buffer || !applespi->tx_status ||
-	    !applespi->rx_buffer || !applespi->msg_buf)
-		return -ENOMEM;
+	    !applespi->rx_buffer || !applespi->msg_buf) {
+		sts = -ENOMEM;
+		goto err_restore_dma;
+	}
 
 	/* set up our spi messages */
 	applespi_setup_read_txfrs(applespi);
@@ -1658,7 +1684,8 @@ static int applespi_probe(struct spi_device *spi)
 		dev_err(&applespi->spi->dev,
 			"Failed to get SIEN ACPI method handle: %s\n",
 			acpi_format_exception(acpi_sts));
-		return -ENODEV;
+		sts = -ENODEV;
+		goto err_restore_dma;
 	}
 
 	acpi_sts = acpi_get_handle(spi_handle, "SIST", &applespi->sist);
@@ -1666,23 +1693,26 @@ static int applespi_probe(struct spi_device *spi)
 		dev_err(&applespi->spi->dev,
 			"Failed to get SIST ACPI method handle: %s\n",
 			acpi_format_exception(acpi_sts));
-		return -ENODEV;
+		sts = -ENODEV;
+		goto err_restore_dma;
 	}
 
 	/* switch on the SPI interface */
 	sts = applespi_setup_spi(applespi);
 	if (sts)
-		return sts;
+		goto err_restore_dma;
 
 	sts = applespi_enable_spi(applespi);
 	if (sts)
-		return sts;
+		goto err_restore_dma;
 
 	/* setup the keyboard input dev */
 	applespi->keyboard_input_dev = devm_input_allocate_device(&spi->dev);
 
-	if (!applespi->keyboard_input_dev)
-		return -ENOMEM;
+	if (!applespi->keyboard_input_dev) {
+		sts = -ENOMEM;
+		goto err_restore_dma;
+	}
 
 	applespi->keyboard_input_dev->name = "Apple SPI Keyboard";
 	applespi->keyboard_input_dev->phys = "applespi/input0";
@@ -1717,7 +1747,7 @@ static int applespi_probe(struct spi_device *spi)
 	if (sts) {
 		dev_err(&applespi->spi->dev,
 			"Unable to register keyboard input device (%d)\n", sts);
-		return -ENODEV;
+		goto err_restore_dma;
 	}
 
 	/*
@@ -1729,7 +1759,8 @@ static int applespi_probe(struct spi_device *spi)
 		dev_err(&applespi->spi->dev,
 			"Failed to obtain GPE for SPI slave device: %s\n",
 			acpi_format_exception(acpi_sts));
-		return -ENODEV;
+		sts = -ENODEV;
+		goto err_restore_dma;
 	}
 	applespi->gpe = (int)gpe;
 
@@ -1740,7 +1771,8 @@ static int applespi_probe(struct spi_device *spi)
 		dev_err(&applespi->spi->dev,
 			"Failed to install GPE handler for GPE %d: %s\n",
 			applespi->gpe, acpi_format_exception(acpi_sts));
-		return -ENODEV;
+		sts = -ENODEV;
+		goto err_restore_dma;
 	}
 
 	applespi->suspended = false;
@@ -1751,7 +1783,8 @@ static int applespi_probe(struct spi_device *spi)
 			"Failed to enable GPE handler for GPE %d: %s\n",
 			applespi->gpe, acpi_format_exception(acpi_sts));
 		acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
-		return -ENODEV;
+		sts = -ENODEV;
+		goto err_restore_dma;
 	}
 
 	/* trigger touchpad setup */
@@ -1789,6 +1822,11 @@ static int applespi_probe(struct spi_device *spi)
 			    &applespi_tp_dim_fops);
 
 	return 0;
+
+err_restore_dma:
+	if (override_dma)
+		spi->controller->can_dma = applespi->original_can_dma;
+	return sts;
 }
 
 static void applespi_drain_writes(struct applespi_data *applespi)
@@ -1813,6 +1851,7 @@ static void applespi_drain_reads(struct applespi_data *applespi)
 static void applespi_remove(struct spi_device *spi)
 {
 	struct applespi_data *applespi = spi_get_drvdata(spi);
+	bool override_dma = applespi_force_pio || dmi_match(DMI_PRODUCT_NAME, "MacBook8,1");
 
 	applespi_drain_writes(applespi);
 
@@ -1822,6 +1861,9 @@ static void applespi_remove(struct spi_device *spi)
 
 	applespi_drain_reads(applespi);
 
+	if (override_dma)
+		spi->controller->can_dma = applespi->original_can_dma;
+
 	debugfs_remove_recursive(applespi->debugfs_root);
 }
 
-- 
2.39.5

[PATCH v3 2/3] Input: applespi - cancel pending work on driver remove

[Shih-Yuan Lee]

During driver removal in applespi_remove(), the managed private data
structure is freed by devres. However, the driver does not cancel the
asynchronous work applespi->work, which registers the touchpad input
device.

This creates a use-after-free (UAF) vulnerability if a pending or
running worker thread attempts to access the private data after the
remove function returns.

Fix this by explicitly calling cancel_work_sync(&applespi->work) in
applespi_remove() before cleanups.

Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/input/keyboard/applespi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index 79e5cb5001c7..fd785dba1174 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1861,6 +1861,8 @@ static void applespi_remove(struct spi_device *spi)
 
 	applespi_drain_reads(applespi);
 
+	cancel_work_sync(&applespi->work);
+
 	if (override_dma)
 		spi->controller->can_dma = applespi->original_can_dma;
 
-- 
2.39.5

[PATCH v3 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open

[Shih-Yuan Lee]

The tp_dim debugfs file is registered synchronously during driver probe
in applespi_probe(). However, the applespi->touchpad_input_dev is
initialized and registered asynchronously in the driver's worker thread.

If a userspace process opens the debugfs file before the worker thread
has completed initialization, applespi_tp_dim_open() will dereference
the NULL applespi->touchpad_input_dev pointer, causing a kernel panic.

Fix this by using smp_load_acquire() to safely load touchpad_input_dev
and return -ENODEV if it is not yet initialized.

Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/input/keyboard/applespi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index fd785dba1174..e3e239f430bd 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -972,12 +972,18 @@ static void applespi_debug_update_dimensions(struct applespi_data *applespi,
 static int applespi_tp_dim_open(struct inode *inode, struct file *file)
 {
 	struct applespi_data *applespi = inode->i_private;
+	struct input_dev *touchpad;
 
 	file->private_data = applespi;
 
+	/* Pairs with smp_store_release in applespi_register_touchpad_device() */
+	touchpad = smp_load_acquire(&applespi->touchpad_input_dev);
+	if (!touchpad)
+		return -ENODEV;
+
 	snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
 		 "0x%.4x %dx%d+%u+%u\n",
-		 applespi->touchpad_input_dev->id.product,
+		 touchpad->id.product,
 		 applespi->tp_dim_min_x, applespi->tp_dim_min_y,
 		 applespi->tp_dim_max_x - applespi->tp_dim_min_x,
 		 applespi->tp_dim_max_y - applespi->tp_dim_min_y);
-- 
2.39.5