Re: [PATCH v2 3/3] Input: applespi - fix NULL pointer dereference in tp_dim open
From: sashiko-bot@kernel.org
Date: 2026-07-11 07:08:02
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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