* Re: [PATCH v2 09/10] Input: matrix_keyboard - replace linux/gpio.h inclusion
From: Dmitry Torokhov @ 2026-07-11 4:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-gpio, Linus Walleij, Bartosz Golaszewski, Arnd Bergmann,
Andrew Lunn, Sebastian Hesselbarth, Gregory Clement, Frank Li,
Robert Jarzmik, Krzysztof Kozlowski, Greg Ungerer,
Thomas Bogendoerfer, Hauke Mehrtens, Rafał Miłecki,
Yoshinori Sato, John Paul Adrian Glaubitz, Dominik Brodowski,
linux-kernel, linux-arm-kernel, linux-samsung-soc, patches,
linux-m68k, linux-mips, linux-sh, linux-input, linux-sunxi,
linux-phy, linux-rockchip, linux-sound, Bartosz Golaszewski
In-Reply-To: <20260710211954.1373336-10-arnd@kernel.org>
On Fri, Jul 10, 2026 at 11:19:54PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> linux/gpio.h is going away, so remove that since the driver already
> includes linux/gpio/consumer.h.
>
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Adjusted the subject (s/replace/remove) and applied, thank you.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 0/1] Input: applespi - force PIO mode on MacBook8,1
From: Shih-Yuan Lee @ 2026-07-11 5:52 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
Hi Dmitry and the linux-input community,
This patch addresses a long-standing issue on the early 2015 12" MacBook
(MacBook8,1) where the SPI-connected keyboard and trackpad intermittently
fail to initialize on boot, and consistently fail to initialize after warm
reboots (soft restarts).
The issue stems from a race condition between the Intel LPSS DMA controller
(dw_dmac) and the host SPI controller (spi-pxa2xx-pci). When DMA mode is
active on this controller, transfers consistently time out (taking exactly
200ms per transfer), leaving the applespi client device in an uninitialized
and unresponsive state.
When the host controller probes before the DMA controller, it gracefully
falls back to PIO mode, which operates flawlessly.
To resolve this reliably without introducing platform-level DMI quirks into
the generic Intel LPSS PCI driver (which might affect non-Apple systems),
this patch introduces a targeted DMI quirk directly within applespi.c. On
MacBook8,1, it overrides the SPI controller's can_dma callback to a custom
helper that always returns false. This forces the controller to use the
rock-solid PIO mode specifically for this device.
Testing shows 100% reliable initialization of the keyboard and trackpad
across multiple soft and hard reboots.
Shih-Yuan Lee (1):
Input: applespi - force PIO mode on MacBook8,1
drivers/input/keyboard/applespi.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
--
2.39.5
^ permalink raw reply
* [PATCH 1/1] Input: applespi - force PIO mode on MacBook8,1
From: Shih-Yuan Lee @ 2026-07-11 5:52 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711055247.5412-1-fourdollars@debian.org>
On MacBook8,1 (early 2015 12" MacBook), the LPSS SPI controller's DMA
handshake and interrupt routing frequently fail or time out after warm
reboots, causing the keyboard and trackpad to become unresponsive and
spamming "SPI transfer timed out" (-110) errors to dmesg.
Address this by introducing a DMI quirk inside the probe function that
detects the MacBook8,1 model and overrides the controller's can_dma
callback to a custom helper that always returns false. This forces the
host controller to fall back to the rock-solid PIO mode.
This is a self-contained device quirk that completely solves the DMA
race condition on MacBook8,1 systems without impacting other MacBooks or
generic Intel platforms.
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/input/keyboard/applespi.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index b5ff71cd5a70..edf81e383612 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -45,6 +45,7 @@
#include <linux/crc16.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
+#include <linux/dmi.h>
#include <linux/efi.h>
#include <linux/input.h>
#include <linux/input/mt.h>
@@ -1605,6 +1606,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;
@@ -1613,6 +1621,15 @@ static int applespi_probe(struct spi_device *spi)
int sts, i;
unsigned long long gpe, usb_status;
+ /*
+ * MacBook8,1's SPI host controller DMA is broken (timeout errors).
+ * Force PIO mode by overriding the controller's can_dma callback.
+ */
+ if (dmi_match(DMI_PRODUCT_NAME, "MacBook8,1")) {
+ dev_info(&spi->dev, "Disabling DMA for MacBook8,1 SPI to force PIO mode\n");
+ spi->controller->can_dma = applespi_can_not_dma;
+ }
+
/* check if the USB interface is present and enabled already */
acpi_sts = acpi_evaluate_integer(spi_handle, "UIST", NULL, &usb_status);
if (ACPI_SUCCESS(acpi_sts) && usb_status) {
--
2.39.5
^ permalink raw reply related
* Re: [PATCH 1/1] Input: applespi - force PIO mode on MacBook8,1
From: sashiko-bot @ 2026-07-11 6:00 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711055247.5412-2-fourdollars@debian.org>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The client driver improperly modifies the shared SPI host controller's `can_dma` callback, causing an execute-after-free vulnerability upon driver unbind or probe failure.
Pre-existing issues:
- [High] Missing `cancel_work_sync(&applespi->work)` during driver removal leads to a Use-After-Free of the managed private data structure.
- [High] The `tp_dim` debugfs file can be opened by userspace before the touchpad input device is fully initialized, causing a NULL pointer dereference.
--
--- Patch [1]: [PATCH 1/1] Input: applespi - force PIO mode on MacBook8,1 ---
commit e15d04076e4b2b62df355b30d858accad86794ac
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 after warm
reboots. Address this by introducing a DMI quirk inside the probe function
that detects the MacBook8,1 model and overrides the controller's can_dma
callback to a custom helper that always returns false.
> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index b5ff71cd5a701..edf81e3836122 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
> @@ -1605,6 +1606,13 @@ static void applespi_save_bl_level(struct applespi_data *applespi,
[ ... ]
> +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;
> @@ -1613,6 +1621,15 @@ static int applespi_probe(struct spi_device *spi)
> int sts, i;
> unsigned long long gpe, usb_status;
>
> + /*
> + * MacBook8,1's SPI host controller DMA is broken (timeout errors).
> + * Force PIO mode by overriding the controller's can_dma callback.
> + */
> + if (dmi_match(DMI_PRODUCT_NAME, "MacBook8,1")) {
> + dev_info(&spi->dev, "Disabling DMA for MacBook8,1 SPI to force PIO mode\n");
> + spi->controller->can_dma = applespi_can_not_dma;
[Severity: High]
Could this overwrite of the shared SPI host controller's can_dma callback
cause a regression by leaving a dangling pointer if the applespi module is
unloaded?
Since applespi_can_not_dma() is a local function inside this driver, won't
subsequent SPI transfers by other devices on this bus attempt to execute
unmapped memory after this module is removed?
> + }
> +
This isn't a bug introduced by this patch, but looking at applespi_remove():
drivers/input/keyboard/applespi.c:applespi_remove() {
...
applespi_drain_reads(applespi);
debugfs_remove_recursive(applespi->debugfs_root);
}
[Severity: High]
Does this teardown path miss a call to cancel_work_sync(&applespi->work)?
Because devres will free the applespi structure when applespi_remove()
returns, could an already pending or running applespi_worker cause a
use-after-free here?
This is also a pre-existing issue, but looking at applespi_tp_dim_open():
drivers/input/keyboard/applespi.c:applespi_tp_dim_open() {
...
snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
"0x%.4x %dx%d+%u+%u\n",
applespi->touchpad_input_dev->id.product,
...
}
[Severity: High]
Can userspace open the tp_dim debugfs file before the asynchronous worker
has finished initializing applespi->touchpad_input_dev?
Since the debugfs file is registered synchronously during applespi_probe(),
could an early read trigger a NULL pointer dereference here?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260711055247.5412-2-fourdollars@debian.org?part=1
^ permalink raw reply
* [PATCH v2 0/3] Input: applespi - fixes for DMA timeout, UAF, and NULL pointer dereference
From: Shih-Yuan Lee @ 2026-07-11 6:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711055247.5412-1-fourdollars@debian.org>
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) as well as two pre-existing, high-severity
UAF/NULL-pointer bugs in the applespi driver.
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 the
MacBook8,1 DMI quirk and its unbind safety, Patch 2 cancels pending work
synchronously on remove (UAF fix), and Patch 3 protects the debugfs open
handler (NULL pointer fix).
Patch 1 introduces a DMI quirk inside applespi.c that overrides the SPI
controller's can_dma callback to a custom helper that always returns false
for MacBook8,1. This forces the controller to use the rock-solid PIO mode
specifically for this device. To prevent an execute-after-free vulnerability
upon module unload or unbind, it preserves and restores the original can_dma
callback.
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 | 38 ++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
--
2.39.5
^ permalink raw reply
* [PATCH v2 1/3] Input: applespi - force PIO mode on MacBook8,1
From: Shih-Yuan Lee @ 2026-07-11 6:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711065415.7396-1-fourdollars@debian.org>
On MacBook8,1 (early 2015 12" MacBook), the LPSS SPI controller's DMA
handshake and interrupt routing frequently fail or time out after warm
reboots, causing the keyboard and trackpad to become unresponsive and
spamming "SPI transfer timed out" (-110) errors to dmesg.
Address this by introducing a DMI quirk inside the probe function that
detects the MacBook8,1 model and overrides the controller's can_dma
callback to a custom helper that always returns false. This forces the
host controller to fall back to the rock-solid PIO mode.
Since we overwrite the shared host controller's can_dma callback, save
the original callback pointer during probe and restore it in the unbind
(remove) path to prevent an execute-after-free vulnerability when the
applespi driver is unloaded.
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/input/keyboard/applespi.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index b5ff71cd5a70..07a910cb8459 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -45,6 +45,7 @@
#include <linux/crc16.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
+#include <linux/dmi.h>
#include <linux/efi.h>
#include <linux/input.h>
#include <linux/input/mt.h>
@@ -431,6 +432,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 +1610,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;
@@ -1788,6 +1800,19 @@ static int applespi_probe(struct spi_device *spi)
debugfs_create_file("tp_dim", 0400, applespi->debugfs_root, applespi,
&applespi_tp_dim_fops);
+ /*
+ * MacBook8,1's SPI host controller DMA is broken (timeout errors).
+ * Force PIO mode by overriding the controller's can_dma callback.
+ *
+ * Since we modify the shared controller's callback, we save the
+ * original pointer and restore it in applespi_remove().
+ */
+ applespi->original_can_dma = spi->controller->can_dma;
+ if (dmi_match(DMI_PRODUCT_NAME, "MacBook8,1")) {
+ dev_info(&spi->dev, "Disabling DMA for MacBook8,1 SPI to force PIO mode\n");
+ spi->controller->can_dma = applespi_can_not_dma;
+ }
+
return 0;
}
@@ -1822,6 +1847,9 @@ static void applespi_remove(struct spi_device *spi)
applespi_drain_reads(applespi);
+ if (applespi->original_can_dma)
+ spi->controller->can_dma = applespi->original_can_dma;
+
debugfs_remove_recursive(applespi->debugfs_root);
}
--
2.39.5
^ permalink raw reply related
* [PATCH v2 2/3] Input: applespi - cancel pending work on driver remove
From: Shih-Yuan Lee @ 2026-07-11 6:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711065415.7396-1-fourdollars@debian.org>
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 07a910cb8459..b6b4d258d0dd 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1847,6 +1847,8 @@ static void applespi_remove(struct spi_device *spi)
applespi_drain_reads(applespi);
+ cancel_work_sync(&applespi->work);
+
if (applespi->original_can_dma)
spi->controller->can_dma = applespi->original_can_dma;
--
2.39.5
^ permalink raw reply related
* [PATCH v2 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open
From: Shih-Yuan Lee @ 2026-07-11 6:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711065415.7396-1-fourdollars@debian.org>
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 b6b4d258d0dd..1f8e4ae90285 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -968,12 +968,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
^ permalink raw reply related
* Re: [PATCH v2 1/3] Input: applespi - force PIO mode on MacBook8,1
From: sashiko-bot @ 2026-07-11 7:04 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711065415.7396-2-fourdollars@debian.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete restoration of `spi->controller->can_dma` leads to an execute-after-free on module unload if the original callback was NULL.
- [High] Architectural violation and data race due to mutating shared SPI host controller, which is applied too late to prevent DMA timeouts.
--
--- Patch [1]: [PATCH v2 1/3] Input: applespi - force PIO mode on MacBook8,1 ---
commit c3d66aabf4cd47b8a9f7af8039fad4fc085aef5f
Author: Shih-Yuan Lee <fourdollars@debian.org>
Input: applespi - force PIO mode on MacBook8,1
This patch introduces a DMI quirk to force PIO mode on MacBook8,1 laptops.
It overrides the shared SPI controller's can_dma callback to avoid timeout
errors during DMA handshakes, and attempts to restore the original callback
when the driver is removed.
[ ... ]
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
[ ... ]
> @@ -1788,6 +1800,19 @@ static int applespi_probe(struct spi_device *spi)
> debugfs_create_file("tp_dim", 0400, applespi->debugfs_root, applespi,
> &applespi_tp_dim_fops);
>
> + /*
> + * MacBook8,1's SPI host controller DMA is broken (timeout errors).
> + * Force PIO mode by overriding the controller's can_dma callback.
> + *
> + * Since we modify the shared controller's callback, we save the
> + * original pointer and restore it in applespi_remove().
> + */
> + applespi->original_can_dma = spi->controller->can_dma;
> + if (dmi_match(DMI_PRODUCT_NAME, "MacBook8,1")) {
> + dev_info(&spi->dev, "Disabling DMA for MacBook8,1 SPI to force PIO mode\n");
> + spi->controller->can_dma = applespi_can_not_dma;
> + }
> +
> return 0;
> }
[Severity: High]
Is this too late to prevent the DMA timeouts?
The override occurs at the very end of applespi_probe(). However, early SPI
transfers triggered earlier in the function via applespi_setup_spi() and
applespi_init(), as well as any reads triggered by ACPI GPE interrupts that
were already enabled, will still attempt to use DMA and fail.
Furthermore, does mutating the shared SPI host controller's method table from
a peripheral client driver cause data races with concurrent SPI core
operations? Typically, controller-level hardware quirks need to be implemented
in the SPI host controller driver itself rather than the client driver to
maintain encapsulation and prevent races.
[ ... ]
> @@ -1822,6 +1847,9 @@ static void applespi_remove(struct spi_device *spi)
>
> applespi_drain_reads(applespi);
>
> + if (applespi->original_can_dma)
> + spi->controller->can_dma = applespi->original_can_dma;
> +
> debugfs_remove_recursive(applespi->debugfs_root);
> }
[Severity: High]
Will this leave a dangling pointer in the host controller if the original
callback was NULL?
If the host controller does not natively provide a can_dma callback,
applespi->original_can_dma will be NULL. In that scenario, this conditional
check evaluates to false and skips the restoration entirely.
Once the applespi module is unloaded and unmapped from memory, the shared
host controller will be left pointing to the freed applespi_can_not_dma
function. Any subsequent SPI transfers on the host controller would then
dereference this freed pointer, resulting in an execute-after-free and kernel
panic.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260711065415.7396-1-fourdollars@debian.org?part=1
^ permalink raw reply
* Re: [PATCH v2 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open
From: sashiko-bot @ 2026-07-11 7:08 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260711065415.7396-4-fourdollars@debian.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Unbounded allocations of touchpad input devices triggered by repeated PACKET_DEV_INFO SPI messages.
- [High] Deadlock in `applespi_drain_reads` during module unload or system suspend after a CRC error.
--
--- Patch [3]: [PATCH v2 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open ---
commit 54174cce6bfb0203a63026dc8e54eb65b7ee620c
Author: Shih-Yuan Lee <fourdollars@debian.org>
Input: applespi - fix NULL pointer dereference in tp_dim open
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, it will dereference the NULL pointer.
This commit uses smp_load_acquire to safely load the pointer and returns
-ENODEV if not initialized.
> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index b6b4d258d0dd5..1f8e4ae902855 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
> @@ -968,12 +968,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);
[Severity: High]
This isn't a bug introduced by this patch, but could this lead to an unbounded
allocation of touchpad input devices?
When the SPI device sends a PACKET_DEV_INFO message,
applespi_handle_cmd_response() unconditionally queues applespi->work:
applespi->rcvd_tp_info = message->tp_info;
schedule_work(&applespi->work);
return;
The worker function applespi_worker() then calls
applespi_register_touchpad_device(), which unconditionally allocates and
registers a new touchpad device:
/* create touchpad input device */
touchpad_input_dev = devm_input_allocate_device(&applespi->spi->dev);
...
/* register input device */
sts = input_register_device(touchpad_input_dev);
This appears to overwrite the applespi->touchpad_input_dev pointer without
freeing or unregistering the previous one.
If the hardware sends multiple info packets, does this cause a resource leak
of devm-managed memory and input devices?
[Severity: High]
This is a pre-existing issue, but is there a potential deadlock during module
unload or system suspend after a CRC error?
In applespi_got_data(), if a SPI packet fails the CRC check and the driver
is not currently draining, the function returns early without clearing
applespi->read_active:
if (!applespi_verify_crc(applespi, applespi->rx_buffer,
APPLESPI_PACKET_SIZE)) {
guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
if (applespi->drain) {
applespi->read_active = false;
applespi->write_active = false;
wake_up_all(&applespi->drain_complete);
}
return;
}
If no further SPI interrupts arrive before a system suspend or rmmod,
applespi_remove() and applespi_suspend() disable GPE interrupts and then call
applespi_drain_reads():
/* disable GPE interrupt */
acpi_disable_gpe(NULL, applespi->gpe);
applespi_drain_reads(applespi);
applespi_drain_reads() will then wait for !applespi->read_active:
wait_event_lock_irq(applespi->drain_complete, !applespi->read_active,
applespi->cmd_msg_lock);
Since interrupts are disabled, no new read can arrive to clear the flag.
Can this result in an unrecoverable deadlock waiting for read_active to clear?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260711065415.7396-1-fourdollars@debian.org?part=3
^ permalink raw reply
* Re: [PATCH v2 2/3] Input: applespi - cancel pending work on driver remove
From: sashiko-bot @ 2026-07-11 7:09 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711065415.7396-3-fourdollars@debian.org>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-after-free of the applespi structure inside the SPI asynchronous read completion callback during driver removal.
- [High] Deadlock/Hang on driver unbind or suspend due to un-cleared read_active flag on CRC error.
- [High] Missing payload length validation leads to reading uninitialized heap memory or old packet data.
- [High] Unbounded allocation and registration of touchpad input devices triggered by untrusted device input.
- [High] Use-after-free if probe fails after input device registration while async SPI writes are in-flight.
- [High] NULL pointer dereference when opening the tp_dim debugfs file before touchpad initialization.
- [Low] Redundant manual assignment of input_dev->dev.parent when using devm_input_allocate_device().
--
--- Patch [2]: [PATCH v2 2/3] Input: applespi - cancel pending work on driver remove ---
commit fee01b08b7f49cc284abe2da246718c6d357db23
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
> index 07a910cb8459f..b6b4d258d0dd5 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
> @@ -1847,6 +1847,8 @@ static void applespi_remove(struct spi_device *spi)
>
> applespi_drain_reads(applespi);
>
[Severity: High]
This isn't a bug introduced by this patch, but does the read completion callback
access the private data structure after it has been freed during driver removal?
applespi_drain_reads() unblocks when applespi_msg_complete() clears read_active:
applespi_msg_complete()
{
...
if (is_read_compl)
applespi->read_active = false;
...
if (applespi->drain && !applespi->write_active)
wake_up_all(&applespi->drain_complete);
}
However, applespi_msg_complete() is called by applespi_async_read_complete()
before it finishes using the applespi structure:
applespi_async_read_complete(void *context)
{
struct applespi_data *applespi = context;
...
applespi_msg_complete(applespi, true, true);
} else {
applespi_got_data(applespi);
}
acpi_finish_gpe(NULL, applespi->gpe);
}
Once wake_up_all() returns, applespi_remove() can finish, which allows devres
to free the applespi structure. Does the final acpi_finish_gpe() call then
dereference a freed applespi->gpe?
[Severity: High]
This is a pre-existing issue, but can the kernel hang indefinitely during
driver unbind if an SPI read packet fails the CRC check?
In applespi_got_data(), if the CRC check fails and drain is false, it returns
early without clearing read_active:
applespi_got_data()
{
...
if (!applespi_verify_crc(applespi, applespi->rx_buffer,
APPLESPI_PACKET_SIZE)) {
guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
if (applespi->drain) {
applespi->read_active = false;
...
}
return;
}
Since read_active remains true, when applespi_remove() later calls
applespi_drain_reads(), will wait_event_lock_irq() wait indefinitely for
read_active to be cleared, resulting in a deadlock?
> + cancel_work_sync(&applespi->work);
> +
[Severity: High]
This is a pre-existing issue, but can the device trigger unbounded memory
allocations and input device registrations?
In applespi_handle_cmd_response(), whenever an INFO packet is received,
it unconditionally schedules the initialization worker:
applespi_handle_cmd_response()
{
if (packet->device == PACKET_DEV_INFO &&
le16_to_cpu(message->type) == 0x1020) {
...
applespi->rcvd_tp_info = message->tp_info;
schedule_work(&applespi->work);
return;
}
...
The worker then allocates and registers a new touchpad input device
using devm_input_allocate_device():
applespi_worker()
-> applespi_register_touchpad_device()
{
touchpad_input_dev = devm_input_allocate_device(&applespi->spi->dev);
...
sts = input_register_device(touchpad_input_dev);
...
If the device repeatedly sends 0x1020 INFO responses, won't this continually
allocate new managed devices and leak the previous ones?
> if (applespi->original_can_dma)
> spi->controller->can_dma = applespi->original_can_dma;
>
[Severity: Low]
This isn't a bug introduced by this patch, but is the manual assignment of
input_dev->dev.parent redundant here?
Looking at applespi_probe(), it uses devm_input_allocate_device(), which
automatically sets the parent to the device passed to it:
applespi_probe()
{
...
applespi->keyboard_input_dev = devm_input_allocate_device(&spi->dev);
...
applespi->keyboard_input_dev->dev.parent = &spi->dev;
...
The same applies to touchpad_input_dev in applespi_register_touchpad_device().
[Severity: High]
This is a pre-existing issue, but do applespi_got_data() and
applespi_handle_cmd_response() read out-of-bounds memory when processing
messages?
applespi_got_data() checks the packet length against sizeof(packet->data):
if (len > sizeof(packet->data)) {
...
goto msg_complete;
}
But when handling the message payload, it casts the data and processes it
without checking if the packet actually contains the full struct:
if (packet->flags == PACKET_TYPE_READ &&
packet->device == PACKET_DEV_KEYB) {
applespi_handle_keyboard_event(applespi, &message->keyboard);
And similarly in applespi_handle_cmd_response():
if (packet->device == PACKET_DEV_INFO &&
le16_to_cpu(message->type) == 0x1020) {
...
applespi->rcvd_tp_info = message->tp_info;
If an undersized packet is received, does this lead to reading uninitialized
buffer memory (stale data) or out-of-bounds memory?
[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk if
applespi_probe() fails after the keyboard input device is registered?
In applespi_probe(), the keyboard input device is registered:
sts = input_register_device(applespi->keyboard_input_dev);
if (sts) {
...
Later in probe, ACPI GPE setup can fail, returning an error:
acpi_sts = acpi_install_gpe_handler(...);
if (ACPI_FAILURE(acpi_sts)) {
...
return -ENODEV;
}
Because the input device was already visible to userspace, an LED event might
have queued an asynchronous SPI write. If probe returns an error, devres will
free the applespi structure, but the in-flight SPI write is not cancelled.
Will the completion callback then access the freed applespi structure?
applespi_async_write_complete()
{
...
if (!applespi_check_write_status(applespi, applespi->wr_m.status)) {
...
[Severity: High]
This is a pre-existing issue, but is there a risk of a NULL pointer dereference
if the tp_dim debugfs file is opened before the touchpad is initialized?
In applespi_probe(), the tp_dim debugfs file is created synchronously:
debugfs_create_file("tp_dim", 0400, applespi->debugfs_root, applespi,
&applespi_tp_dim_fops);
However, applespi->touchpad_input_dev is allocated asynchronously in
applespi_worker(). If userspace opens the file before the worker executes
(or if the hardware never responds), applespi_tp_dim_open() will dereference
a NULL touchpad_input_dev:
static int applespi_tp_dim_open(struct inode *inode, struct file *file)
{
...
snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
"0x%.4x %dx%d+%u+%u\n",
applespi->touchpad_input_dev->id.product,
...
Could this result in a kernel panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260711065415.7396-1-fourdollars@debian.org?part=2
^ permalink raw reply
* [PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind
From: Doruk Tan Ozturk @ 2026-07-11 7:30 UTC (permalink / raw)
To: jikos, bentiss
Cc: spbnick, linux-input, linux-kernel, stable, Doruk Tan Ozturk
uclogic_remove() drained the in-range emulation timer before calling
hid_hw_stop():
timer_delete_sync(&drvdata->inrange_timer);
hid_hw_stop(hdev);
The timer is re-armed from uclogic_raw_event_pen() with a 100 ms
timeout on every pen-in-range report via mod_timer(), and its callback
uclogic_inrange_timeout() dereferences drvdata to deliver a synthetic
BTN_TOOL_PEN release.
drvdata is allocated with devm_kzalloc(), so the HID core devm cleanup
frees it once uclogic_remove() returns. Because the timer is drained
before hid_hw_stop(), a pen report still completing inside
hid_hw_stop() can reach uclogic_raw_event_pen() and re-arm the timer
after the drain. devm then frees drvdata and the re-armed timer fires
on freed memory, a UAF read in uclogic_inrange_timeout(). This is a
disconnect race (USB unplug or rmmod).
Fix by mirroring the letsketch fix: call hid_hw_stop() first, which
synchronously kills the URBs that deliver raw_event(), so once it
returns no path can re-arm the timer. timer_shutdown_sync() then
drains any in-flight callback and permanently disables further
mod_timer() calls.
Found by 0sec (https://0sec.ai) using automated source analysis, as an
incomplete-fix twin of commit 46c8beeccd8a ("HID: letsketch: fix UAF on
inrange_timer at driver unbind"); not runtime-reproduced.
Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
drivers/hid/hid-uclogic-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..c440013a609f 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -548,8 +548,8 @@ static void uclogic_remove(struct hid_device *hdev)
{
struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
- timer_delete_sync(&drvdata->inrange_timer);
hid_hw_stop(hdev);
+ timer_shutdown_sync(&drvdata->inrange_timer);
kfree(drvdata->desc_ptr);
uclogic_params_cleanup(&drvdata->params);
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 06/10] ASoC: replace linux/gpio.h inclusions
From: Sergey Shtylyov @ 2026-07-11 7:34 UTC (permalink / raw)
To: Arnd Bergmann, linux-gpio, Linus Walleij, Bartosz Golaszewski
Cc: Arnd Bergmann, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Frank Li, Robert Jarzmik, Krzysztof Kozlowski,
Greg Ungerer, Thomas Bogendoerfer, Hauke Mehrtens,
Rafał Miłecki, Yoshinori Sato,
John Paul Adrian Glaubitz, Dmitry Torokhov, Dominik Brodowski,
linux-kernel, linux-arm-kernel, linux-samsung-soc, patches,
linux-m68k, linux-mips, linux-sh, linux-input, linux-sunxi,
linux-phy, linux-rockchip, linux-sound, Bartosz Golaszewski
In-Reply-To: <20260710211954.1373336-7-arnd@kernel.org>
On 7/11/26 12:19 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> linux/gpio.h is going away,s o use linux/gpio/consumer.h instead.
s/s o/ so/?
>
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>[...]
MBR, Sergey
^ permalink raw reply
* Re: [PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind
From: sashiko-bot @ 2026-07-11 7:41 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711073003.71012-1-doruk@0sec.ai>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Moving timer_shutdown_sync() after hid_hw_stop() introduces a Use-After-Free of drvdata->pen_input during driver unbind.
Pre-existing issues:
- [Critical] Missing timer cancellation in the uclogic_probe() error path leads to a Use-After-Free if initialization fails after I/O has started.
--
--- Patch [1]: [PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind ---
commit 0de50e5b91e3a9978c96d85e0aab45d43b0d98d6
Author: Doruk Tan Ozturk <doruk@0sec.ai>
HID: uclogic: fix UAF on inrange_timer at driver unbind
uclogic_remove() drained the in-range emulation timer before calling
hid_hw_stop(), leaving a window where a completing pen report could
re-arm the timer. This commit reorders the operations to call
hid_hw_stop() first, preventing re-arming before shutting down the timer.
> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index b73f09d26688a..c440013a609fd 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -548,8 +548,8 @@ static void uclogic_remove(struct hid_device *hdev)
> {
> struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
>
> - timer_delete_sync(&drvdata->inrange_timer);
> hid_hw_stop(hdev);
> + timer_shutdown_sync(&drvdata->inrange_timer);
[Severity: Critical]
Can moving timer_shutdown_sync() after hid_hw_stop() introduce a
use-after-free on drvdata->pen_input?
hid_hw_stop() synchronously unregisters and frees the standard input
devices, which frees the memory pointed to by drvdata->pen_input.
If an incoming report arms the timer just before hid_hw_stop() is called,
the timer will remain active.
When it fires, the callback will unconditionally dereference the freed
pointer:
drivers/hid/hid-uclogic-core.c:uclogic_inrange_timeout()
struct input_dev *input = drvdata->pen_input;
if (input == NULL)
return;
input_report_abs(input, ABS_PRESSURE, 0);
[Severity: Critical]
This is a pre-existing issue, but can the uclogic_probe() error path
also lead to a use-after-free if initialization fails?
If hid_hw_start() briefly starts hardware I/O but subsequently fails
internally, an incoming report could trigger uclogic_raw_event_pen() and
arm the timer before the error path is taken:
drivers/hid/hid-uclogic-core.c:uclogic_probe()
rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (rc) {
hid_err(hdev, "hw start failed\n");
goto failure;
}
return 0;
failure:
/* Assume "remove" might not be called if "probe" failed */
if (params_initialized)
uclogic_params_cleanup(&drvdata->params);
return rc;
Because this cleanup block does not cancel the inrange_timer, the devm
framework will free the drvdata structure upon probe return.
This leaves the timer armed to execute on freed memory.
> kfree(drvdata->desc_ptr);
> uclogic_params_cleanup(&drvdata->params);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260711073003.71012-1-doruk@0sec.ai?part=1
^ permalink raw reply
* [PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error
From: Doruk Tan Ozturk @ 2026-07-11 8:03 UTC (permalink / raw)
To: jikos, bentiss
Cc: spbnick, linux-input, linux-kernel, stable, Doruk Tan Ozturk
In-Reply-To: <20260711073003.71012-1-doruk@0sec.ai>
uclogic_probe() arms a per-device timer whose callback
uclogic_inrange_timeout() dereferences drvdata->pen_input, and
uclogic_raw_event_pen() re-arms it with a 100 ms timeout on every
in-range pen report.
uclogic_remove() drained the timer with timer_delete_sync() before
hid_hw_stop(). timer_delete_sync() does not block re-arming: a pen
report delivered before hid_hw_stop() kills the URBs can re-arm the timer
after it was drained. hid_hw_stop() then frees the hidinput pen_input
(via hidinput_disconnect() -> input_unregister_device()), and the pending
timer fires on freed memory.
Use timer_shutdown_sync() instead, still before hid_hw_stop(). It drains
the callback while pen_input is still valid and permanently blocks
re-arming, so an in-flight raw_event cannot revive the timer; hid_hw_stop()
then frees pen_input with the timer already dead.
The probe error path had the same exposure: if hid_hw_start() started I/O
and then failed, raw_event may have armed the timer, which would fire on
the devm-freed drvdata after probe returns. Shut the timer down there too.
Unlike letsketch, whose input devices are devm-allocated and outlive
hid_hw_stop(), uclogic's pen_input is freed inside hid_hw_stop(), so the
timer must be shut down before it rather than after.
Found by 0sec (https://0sec.ai) using automated source analysis; not
runtime-reproduced.
Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v2:
- Shut the timer down *before* hid_hw_stop() rather than after. v1
mirrored the letsketch ordering (hid_hw_stop() first), but uclogic's
pen_input is the hidinput device freed inside hid_hw_stop(), not a
devm device that outlives it as in letsketch. A timer armed just
before the URBs are killed could still fire on the freed pen_input in
the window before timer_shutdown_sync() drained it. Running
timer_shutdown_sync() before hid_hw_stop() drains the callback while
pen_input is still valid and blocks re-arming, closing that window.
- Also shut the timer down on the probe error path.
drivers/hid/hid-uclogic-core.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..d74f98efa879 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -267,6 +267,13 @@ static int uclogic_probe(struct hid_device *hdev,
/* Assume "remove" might not be called if "probe" failed */
if (params_initialized)
uclogic_params_cleanup(&drvdata->params);
+ /*
+ * If hid_hw_start() started I/O and then failed, raw_event may have
+ * armed the timer; shut it down so it cannot fire on the devm-freed
+ * drvdata after probe returns.
+ */
+ if (drvdata)
+ timer_shutdown_sync(&drvdata->inrange_timer);
return rc;
}
@@ -548,7 +555,15 @@ static void uclogic_remove(struct hid_device *hdev)
{
struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
- timer_delete_sync(&drvdata->inrange_timer);
+ /*
+ * timer_delete_sync() does not prevent re-arming, so a pen report
+ * delivered before hid_hw_stop() kills the URBs could re-arm the
+ * timer; hid_hw_stop() then frees the hidinput pen_input and the
+ * pending timer fires on freed memory. timer_shutdown_sync() drains
+ * the callback while pen_input is still valid and permanently blocks
+ * re-arming, so an in-flight raw_event cannot revive it.
+ */
+ timer_shutdown_sync(&drvdata->inrange_timer);
hid_hw_stop(hdev);
kfree(drvdata->desc_ptr);
uclogic_params_cleanup(&drvdata->params);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error
From: sashiko-bot @ 2026-07-11 8:37 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711080350.81108-1-doruk@0sec.ai>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing kfree(drvdata->desc_ptr) in the uclogic_probe error path leads to a memory leak of the dynamically allocated replacement report descriptor.
--
--- Patch [1]: [PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error ---
commit 765009419dfdd98b6d556b9c30dc95cac5ee9252
Author: Doruk Tan Ozturk <doruk@0sec.ai>
HID: uclogic: fix UAF on inrange_timer at teardown and probe error
This commit replaces timer_delete_sync() with timer_shutdown_sync() to
prevent use-after-free bugs caused by the timer being re-armed during device
teardown. It also ensures the timer is properly shut down in the probe error
path.
> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -267,6 +267,13 @@ static int uclogic_probe(struct hid_device *hdev,
> /* Assume "remove" might not be called if "probe" failed */
> if (params_initialized)
> uclogic_params_cleanup(&drvdata->params);
> + /*
> + * If hid_hw_start() started I/O and then failed, raw_event may have
> + * armed the timer; shut it down so it cannot fire on the devm-freed
> + * drvdata after probe returns.
> + */
> + if (drvdata)
> + timer_shutdown_sync(&drvdata->inrange_timer);
[Severity: High]
This isn't a bug introduced by this patch, but does this error path leak
drvdata->desc_ptr?
If uclogic_probe() successfully allocates drvdata->desc_ptr via
uclogic_params_get_desc(), but a subsequent initialization step like
hid_parse() or hid_hw_start() fails, it jumps to the failure label. The
error path cleans up params and shuts down the timer, but it appears to miss
calling kfree(drvdata->desc_ptr) before returning.
> return rc;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260711080350.81108-1-doruk@0sec.ai?part=1
^ permalink raw reply
* [PATCH v3 0/3] Input: applespi - fixes for DMA timeout, UAF, and NULL pointer dereference
From: Shih-Yuan Lee @ 2026-07-11 9:20 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, 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
^ permalink raw reply
* [PATCH v3 1/3] Input: applespi - force PIO mode on MacBook8,1
From: Shih-Yuan Lee @ 2026-07-11 9:20 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711092054.13818-1-fourdollars@debian.org>
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
^ permalink raw reply related
* [PATCH v3 2/3] Input: applespi - cancel pending work on driver remove
From: Shih-Yuan Lee @ 2026-07-11 9:20 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711092054.13818-1-fourdollars@debian.org>
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
^ permalink raw reply related
* [PATCH v3 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open
From: Shih-Yuan Lee @ 2026-07-11 9:20 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711092054.13818-1-fourdollars@debian.org>
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
^ permalink raw reply related
* Re: [PATCH v3 1/3] Input: applespi - force PIO mode on MacBook8,1
From: sashiko-bot @ 2026-07-11 9:30 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711092054.13818-2-fourdollars@debian.org>
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
^ permalink raw reply
* Re: [PATCH v3 2/3] Input: applespi - cancel pending work on driver remove
From: sashiko-bot @ 2026-07-11 9:36 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711092054.13818-3-fourdollars@debian.org>
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
^ permalink raw reply
* Re: [PATCH v3 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open
From: sashiko-bot @ 2026-07-11 9:39 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260711092054.13818-4-fourdollars@debian.org>
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
^ permalink raw reply
* Re: [RFC] Input: matrix_keypad: fix interrupt regression introduced by commit 01c84b03d80a
From: Siarhei Volkau @ 2026-07-11 10:59 UTC (permalink / raw)
To: Paul Cercueil
Cc: Alexander Kochetkov, linux-input, Dmitry Torokhov, linux-kernel
In-Reply-To: <CAKNVLfbK5TEFSRuJSv-3Bvx+JjLuPTn1LjKj7nBEEdGxsqiQjg@mail.gmail.com>
> For testing, you could temporarily modify the driver
I've done the test on all JZ47xx I have and it turns out that
my assumption was wrong. All of them report live state of
the pin.
So, guys, the patch can be reverted with no doubts.
BR,
Siarhei
чт, 9 июл. 2026 г. в 18:51, Siarhei Volkau <lis8215@gmail.com>:
>
> > Siarhei, do you remember the context of that patch?
>
> Yes. My understanding is that at least some Ingenic GPIO controllers
> don't report the current input level while the pin is configured for IRQs,
> but instead expose the level latched at the moment the interrupt occurred.
> That's what I was trying to work around.
>
> > That was definitely working on the SoC I tested ...
>
> You wouldn't notice this with the gpio-keys driver because its usage
> pattern matches this behavior. For example, if the IRQ is configured
> for the falling edge, then when the interrupt fires the level register will
> read 0, which is exactly what gpio-keys expects.
>
> I'm not sure whether this affects all Ingenic GPIO controllers,
> but it should be easy to verify. For testing, you could temporarily
> modify the driver as follows:
>
> In the IRQ handler, before reconfiguring the IRQ for the opposite edge:
> - read and print the GPIO level register/bit;
> - wait for an arbitrary amount of time (long enough to release the
> button between the two reads);
> - read and print the GPIO level register/bit again.
>
> If both reads return 0, even though the button has been released,
> then the controller behaves like the JZ4755 and latches the GPIO
> level at the time of the interrupt.
> If the first read returns 0 and the second returns 1, then the controller
> reports the live GPIO input level, as typical GPIO controllers do.
>
> > I wonder if you were just experiencing bouncing?
>
> I'm sure no, I remember that without that patch, I got the same readings
> for all buttons in a row, no matter which column was active and as far as I
> remember I tested it with arbitrary long col-scan-delay-us in range of
> 10-100 ms.
>
> BR,
> Siarhei
>
> чт, 9 июл. 2026 г. в 13:25, Paul Cercueil <paul@crapouillou.net>:
> >
> > Hi,
> >
> > Siarhei, do you remember the context of that patch?
> >
> > The driver will read the pin level to emulate pins configured in "both
> > edges" IRQ mode, and switch to "rising edge" if low, and "falling edge"
> > if high. That was definitely working on the SoC I tested it with
> > (probably JZ4770). I would be surprised if it did not work on the
> > JZ4755.
> >
> > You mention in that commit message that you would get the pin level
> > before the interrupt happened, I wonder if you were just experiencing
> > bouncing?
> >
> > Anyway, I agree that this commit can be reverted.
> >
> > Le mercredi 08 juillet 2026 à 18:59 +0300, Siarhei Volkau a écrit :
> > > Regarding JZ4755, I agree that the [1] patch can be reverted.
> > > No device requiring this workaround has landed in the mainline.
> > > qi,lb60 (Ben Nanonote) seems unaffected as it was there before
> > > the patch was proposed.
> > >
> > > However, if there are genuinely two hardware operating modes:
> > > - "GPIO input mode" while scanning
> > > - "GPIO interrupt input mode" while idle
> > > then the keypad driver should be aware of this distinction.
> > >
> > > I'd like to propose pinctrl state transitions as the mechanism for
> > > switching
> > > these modes, wherever it makes sense (Allwinner et al).
> >
> > I agree.
> >
> > >
> > > CC Paul as the maintainer of Ingenic pinctrl driver.
> > >
> > > BR,
> > > Siarhei
> >
> > Cheers,
> > -Paul
> >
> > >
> > >
> > > ср, 8 июл. 2026 г. в 16:30, Alexander Kochetkov
> > > <al.kochet@gmail.com>:
> > > >
> > > > Hi everyone,
> > > >
> > > > I found a commit [1] that breaks the matrix_keypad behavior on the
> > > > Allwinner A64.
> > > >
> > > > I have a PINE A64-LTS board with a connected keypad polled by
> > > > matrix_keypad.
> > > > This commit caused interrupts to stop arriving at the matrix_keypad
> > > > driver entirely.
> > > >
> > > > An explicit call to gpiod_direction_input() disables interrupt
> > > > reception on all chips
> > > > where the GPIO input mode and GPIO interrupt input mode are
> > > > configured via
> > > > the pinmux register. Interrupt reception is guaranteed to break on
> > > > Allwinner
> > > > (A64, H3, H6, etc.), Broadcom (BCM2835/2711), and some Rockchip
> > > > SoCs. It
> > > > does not break on NXP i.MX (i.MX6, i.MX8), STMicroelectronics
> > > > (STM32MP1),
> > > > TI Sitara (AM335x), or Intel/AMD.
> > > >
> > > > Furthermore, the assumption that enable_row_irqs() restores the
> > > > interrupt mode is
> > > > also specific to the Ingenic pinctrl. In the vast majority of
> > > > drivers, enable_row_irqs() is
> > > > supposed to simply set the interrupt enable mask without changing
> > > > the pinmux.
> > > >
> > > > Commit [1] was introduced to work around a hardware limitation in
> > > > Ingenic's JZ4755.
> > > > This specific behavior is unique to Ingenic and a small number of
> > > > specialized chips.
> > > > The majority of SoCs (around 90%) allow reading a GPIO input that
> > > > is currently configured
> > > > as an interrupt source.
> > > >
> > > > In my opinion, the correct approach would be to revert this commit
> > > > and fix the behavior of
> > > > ingenic_gpio_get_value() inside pinctrl-ingenic.c instead. However,
> > > > I do not own an Ingenic
> > > > board, so I won't be able to test such a patch.
> > > >
> > > > Alternatively, I could introduce a DTS property like read-gpio-
> > > > quirk in matrix_keypad to restore
> > > > the original driver behavior by default, and add this property to
> > > > qi_lb60.dts so as not to break
> > > > the Ingenic platform.
> > > >
> > > > What do you think? What is the best way to proceed here?
> > > >
> > > > Best regards,
> > > > Alexander Kochetkov
> > > >
> > > > [1] commit 01c84b03d80aab9f04c4e3e1f9085f4202ff7c29 ("Input:
> > > > matrix_keypad - force switch rows to input mode")
^ permalink raw reply
* [PATCH v4 0/3] Input/SPI: fixes for MacBook8,1 DMA timeout, UAF, and NULL pointer dereference
From: Shih-Yuan Lee @ 2026-07-11 11:49 UTC (permalink / raw)
To: Dmitry Torokhov, Mark Brown
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Shih-Yuan Lee,
linux-input, linux-spi, linux-kernel, linux-arm-kernel
In-Reply-To: <20260711055247.5412-1-fourdollars@debian.org>
Hi Dmitry, Mark, and the linux-input/linux-spi 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 v4:
- Reverted the runtime `can_dma` callback override from `applespi.c` to avoid
architectural layering violation, data races (TOCTOU) with concurrent SPI
transfers, and execute-after-free vulnerability upon module unload.
- Moved the DMI quirk forcing PIO mode for MacBook8,1 to the host PCI glue
driver (spi-pxa2xx-pci.c) where LPSS setup occurs.
- Formatted the DMI match using a structured `pxa2xx_spi_pci_dmi_table` and
helper function `pxa2xx_spi_pci_can_dma()`.
- Added a `force_pio` module parameter in spi-pxa2xx-pci.c to allow other
users to manually force PIO mode for debugging.
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.
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 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 2 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.
Patch 3 introduces a structured DMI quirk and a `force_pio` module parameter
in spi-pxa2xx-pci.c to disable DMA on MacBook8,1. This forces the controller
to use the rock-solid PIO mode from the very beginning.
Best regards,
Shih-Yuan Lee
Shih-Yuan Lee (3):
Input: applespi - cancel pending work on driver remove
Input: applespi - fix NULL pointer dereference in tp_dim open
spi: pxa2xx: disable DMA for Apple MacBook8,1
drivers/input/keyboard/applespi.c | 10 +++++++++-
drivers/spi/spi-pxa2xx-pci.c | 33 ++++++++++++++++++++++++++++++-
2 files changed, 41 insertions(+), 2 deletions(-)
--
2.39.5
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox