[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The child device driver (`applespi`) overwrites the parent SPI controller's `can_dma` callback, causing an architectural layering violation, data races (TOCTOU) with concurrent SPI transfers, and potential execute-after-free vulnerabilities upon module unload.

Pre-existing issues:
- [High] Race condition between manual cleanup and devres unregistration leads to a Use-After-Free via `applespi->work`.
--

--- Patch [1]: [PATCH v3 1/3] Input: applespi - force PIO mode on MacBook8,1 ---
commit fb42db4427f33df07132a7b2bd913031f55465ea
Author: Shih-Yuan Lee <fourdollars@debian.org>

Input: applespi - force PIO mode on MacBook8,1

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.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331

> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
[ ... ]
> @@ -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;

[Severity: High]
Is it safe for a child device driver to overwrite the parent SPI controller's
shared can_dma callback?

Modifying spi->controller->can_dma directly without a controller-wide lock
could cause a data race if the SPI worker thread concurrently dereferences
ctlr->can_dma for another device on the same bus.

> +	}
> +
>  	INIT_WORK(&applespi->work, applespi_worker);
[ ... ]
> @@ -1822,6 +1861,9 @@ static void applespi_remove(struct spi_device *spi)
>  
>  	applespi_drain_reads(applespi);
>  
>  	cancel_work_sync(&applespi->work);

[Severity: High]
This is a pre-existing issue, but does this create a use-after-free window
during module unload?

In applespi_remove(), the work is canceled, but the input and LED devices
are managed by devm_ and aren't unregistered until the devres release phase
after applespi_remove() completes. 

During this window, if a user writes to the LED sysfs interface, it will
call applespi_set_bl_level() which calls schedule_work(&applespi->work),
re-arming the worker. When devres subsequently frees the applespi structure,
the queued work might execute on freed memory.

> +	if (override_dma)
> +		spi->controller->can_dma = applespi->original_can_dma;

[Severity: High]
Could restoring the can_dma callback without synchronization leave a dangling
pointer if multiple instances bind or unbind?

The restore happens conditionally based on a dynamically recalculated
override_dma boolean. If the module is unloaded but fails to restore the
pointer properly, a dangling pointer will remain in the shared SPI controller.
The SPI core could then invoke the freed applespi_can_not_dma function inside
the unloaded module, resulting in an execute-after-free.

> +
>  	debugfs_remove_recursive(applespi->debugfs_root);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711092054.13818-1-fourdollars@debian.org?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free of `applespi` structure in SPI async read completion callback
- [High] Unbounded wait in `applespi_drain_writes` leading to deadlock on driver unbind or suspend
- [High] NULL pointer dereference in debugfs 'tp_dim' open handler
- [High] CRC error in SPI packet permanently leaks `read_active` flag, causing deadlock
- [High] Unsafe DMA allocation using `devm_kmalloc` leads to cacheline corruption
--

--- Patch [2]: [PATCH v3 2/3] Input: applespi - cancel pending work on driver remove ---
commit 600aa576cb85623de00e20bade1177db921cccac
Author: Shih-Yuan Lee <fourdollars@debian.org>

Input: applespi - cancel pending work on driver remove
    
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.

> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> --- 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);

[Severity: High]
This is a pre-existing issue, but does applespi_drain_reads() create a race
condition with the SPI async read completion callback?

If applespi_drain_reads() waits for read_active to become false, and the
callback applespi_async_read_complete() clears this flag via
applespi_msg_complete(), it unblocks the removal thread immediately:

applespi_async_read_complete() {
    ...
    applespi_msg_complete(applespi, ...);
    ...
    acpi_finish_gpe(NULL, applespi->gpe);
}

Can the removal thread finish applespi_remove() and free the applespi
structure via devres before the completion callback finishes executing,
leading to a use-after-free when acpi_finish_gpe() is called?

[Severity: High]
This is a pre-existing issue, but can this unbind or suspend path deadlock
due to unbounded waits?

Looking at applespi_drain_writes() called earlier in this path, it waits
for write_active to become false:

applespi_drain_writes() {
    applespi->drain = true;
    wait_event_lock_irq(applespi->drain_complete, !applespi->write_active,
                        applespi->cmd_msg_lock);
}

If the hardware fails to respond to a command, write_active remains true.
Because drain is set to true, the lazy timeout logic in
applespi_send_cmd_msg() is unreachable. Does this wait_event block
indefinitely without a fallback timeout if a write response is never received?

> +	cancel_work_sync(&applespi->work);
> +
>  	if (override_dma)
>  		spi->controller->can_dma = applespi->original_can_dma;
>  

[ ... ]

[Severity: High]
This is a pre-existing issue, but is there a risk of a NULL pointer
dereference when accessing the tp_dim debugfs file?

The tp_dim debugfs file is created synchronously during probe, but
applespi->touchpad_input_dev is allocated asynchronously in a workqueue.
If a user opens the debugfs file before the workqueue executes (or if it
never executes due to a hardware failure), applespi_tp_dim_open() will see
a NULL pointer:

applespi_tp_dim_open() {
    ...
    applespi->touchpad_input_dev->id.product,
    ...
}

Can this unconditionally dereference touchpad_input_dev while it is NULL?

[Severity: High]
This is a pre-existing issue, but does a CRC error permanently leak the
read_active state?

In applespi_got_data(), if a packet fails CRC validation and drain is
false, it returns early:

applespi_got_data() {
    if (!applespi_verify_crc(...)) {
        if (applespi->drain) {
            ...
        }
        return;
    }
    ...
}

This skips calling applespi_msg_complete(), leaving read_active set to
true. When the system later suspends or the driver is unbound, will
applespi_drain_reads() wait indefinitely for read_active to become
false, causing a deadlock?

[Severity: High]
This is a pre-existing issue, but are the SPI buffers safe for DMA transfers?

In applespi_probe(), the buffers are allocated using devm_kmalloc():

applespi_probe() {
    ...
    applespi->tx_buffer = devm_kmalloc(&spi->dev, APPLESPI_PACKET_SIZE, ...);
    applespi->rx_buffer = devm_kmalloc(&spi->dev, APPLESPI_PACKET_SIZE, ...);
    ...
}

Since devm allocations prepend a struct devres header immediately before
the returned pointer, the devres header shares the same cacheline as the
start of the buffer.

When the SPI core maps these buffers for DMA, will DMA cache invalidation
(especially DMA_FROM_DEVICE for Rx) invalidate the CPU cacheline and
corrupt the devres linked list headers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711092054.13818-1-fourdollars@debian.org?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Repeated processing of expected or malicious hardware `PACKET_DEV_INFO` responses causes unbounded allocation and registration of `devm`-managed input devices.
- [High] Use-After-Free of `applespi` private data in the asynchronous SPI read callback during driver unbind.
- [High] Missing bounds check on keyboard release events causes an out-of-bounds read of kernel memory, leading to an information leak.
--

--- Patch [3]: [PATCH v3 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open ---
commit 44778c849d055105696c2f1ad69bc1cf09ff47d0
Author: Shih-Yuan Lee <fourdollars@debian.org>

Input: applespi - fix NULL pointer dereference in tp_dim open

This commit fixes a NULL pointer dereference in applespi_tp_dim_open()
when userspace accesses the debugfs file before asynchronous initialization
completes. It uses smp_load_acquire() to safely load the touchpad input
device and returns -ENODEV if not initialized.

> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index fd785dba1174e..e3e239f430bd9 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;
> +

[Severity: High]
This isn't a bug introduced by this patch, but does the driver leak resources
if it repeatedly processes hardware device info responses?

If applespi_handle_cmd_response() receives this packet, it schedules the
worker:

applespi_handle_cmd_response() {
    ...
    schedule_work(&applespi->work);
}

The worker unconditionally calls applespi_register_touchpad_device(), which
allocates a new devm-managed device:

applespi_register_touchpad_device() {
    ...
    touchpad_input_dev = devm_input_allocate_device(&applespi->spi->dev);
    ...
}

Could this lead to unbounded allocation and registration of devm-managed input
devices, losing the reference to the old ones and eventually exhausting memory?

[Severity: High]
This is a pre-existing issue, but is there a potential use-after-free in the
asynchronous SPI read callback during driver unbind?

During driver removal, applespi_remove() calls applespi_drain_reads(), which
waits for read_active to become false:

applespi_remove() {
    ...
    applespi_drain_reads(applespi);
    ...
}

In applespi_async_read_complete(), applespi_msg_complete() sets read_active
to false and wakes the waitqueue, then proceeds to use applespi->gpe:

applespi_async_read_complete() {
    ...
    applespi_msg_complete(applespi, false, false);
    ...
    acpi_finish_gpe(NULL, applespi->gpe);
}

If devres cleanup frees the applespi struct immediately after
applespi_drain_reads() returns, could accessing applespi->gpe cause a
use-after-free?

[Severity: High]
This isn't a bug introduced by this patch, but is there an out-of-bounds read
when processing keyboard release events?

In applespi_handle_keyboard_event(), the unvalidated keys_pressed array is
copied directly into last_keys_pressed:

applespi_handle_keyboard_event() {
    ...
    memcpy(&applespi->last_keys_pressed, keyboard_protocol->keys_pressed,
           sizeof(applespi->last_keys_pressed));
    ...
}

On a subsequent keyboard event, if a previously stored invalid keycode is
processed as released, it is passed to applespi_code_to_key():

applespi_handle_keyboard_event() {
    ...
    key = applespi_code_to_key(applespi->last_keys_pressed[i],
                               applespi->last_keys_fn_pressed[i]);
    ...
}

applespi_code_to_key() uses this unvalidated keycode to index the
applespi_scancodes array:

applespi_code_to_key() {
    ...
    unsigned int key = applespi_scancodes[code];
    ...
}

Could a maliciously crafted SPI packet cause an out-of-bounds read of
applespi_scancodes leading to an information leak?

>  	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);
> 
>  	return nonseekable_open(inode, file);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711092054.13818-1-fourdollars@debian.org?part=3