[PATCH v3 0/5] Input: applespi - Fix probe timeout and use-after-free bugs

[PATCH v3 0/5] Input: applespi - Fix probe timeout and use-after-free bugs

[Shih-Yuan Lee]

This 5-patch series refactors the applespi driver for Apple SPI keyboards and touchpads,
eliminating use-after-free (UAF) race conditions during driver unbind, kernel panics
in debugfs, level-triggered ACPI GPE interrupt storms, and data races.

Specifically, this series:
  - Replaces separate drain helpers with a unified wait queue and 3-second timeout barrier,
    falling back to unconditional completion waiting if transfers remain active to prevent
    premature driver unbind and DMA memory corruption.
  - Tracks in-flight asynchronous SPI transfers using a completion tracking array and
    cancel_spi flag, ensuring inner completion callbacks execute prior to waking wait
    queue waiters to guarantee teardown safety.
  - Converts touchpad detection and registration from an asynchronous workqueue worker
    to a synchronous probe sequence, while protecting against NULL pointer dereferences in
    debugfs during keyboard-only fallback mode.
  - Corrects driver unbind sequence by disabling ACPI GPE handlers before marking the driver
    as shutting down, avoiding level-triggered interrupt storms on unhandled reads.

Changes in v3:
  - Addressed feedback from Sashiko review on the v2 patchset:
  - Fixed Use-After-Free in applespi_drain_writes() and applespi_drain_reads()
    on timeout by falling back to unconditional wait (wait_event_lock_irq()) if
    transfers are still active upon 3-second timeout expiry.
  - Activated cancel_spi tracking flag by setting cancel_spi = true in drain functions
    under cmd_msg_lock to reject new async requests (-ESHUTDOWN) and trigger wait
    queue wakeups.
  - Fixed completion callback race in applespi_async_complete() by invoking inner
    complete(applespi) callback BEFORE clearing tracking slot and waking waiters
    on wait_queue.
  - Fixed NULL pointer dereference in applespi_tp_dim_open() when touchpad registration
    fails or times out, returning -ENODEV if touchpad_input_dev is NULL.
  - Protected rcvd_tp_info from data races in probe by making a safe local copy under
    cmd_msg_lock before passing to applespi_register_touchpad_device().
  - Fixed GPE Interrupt Storm during unbind in applespi_remove() by disabling and
    removing GPE handler BEFORE setting cancel_spi = true and waiting for transfers to drain.

Changes since v1:
  - Split the large touchpad registration patch into 4 distinct, single-purpose
    commits (wait queue consolidation & timeouts, async queue slots tracking,
    synchronous registration, and async probe preference) for better readability.
  - Fixed a self-deadlock in applespi_async() where it attempted to acquire
    cmd_msg_lock while already held by applespi_notify() and other callers.
  - Updated applespi_async() to assert the lock is held by the caller.
  - Protected touchpad info flags under cmd_msg_lock in response handling.
  - Simplified applespi_tp_dim_open() by removing redundant NULL checks.
  - Overhauled GPE disabling and teardown order in applespi_remove() to prevent
    GPE interrupt storms and unbind deadlocks.
  - Addressed reviewer feedback from Sashiko.

Shih-Yuan Lee (5):
  Input: applespi - use unified wait queue with timeouts for drain
  Input: applespi - track asynchronous SPI transfers in flight
  Input: applespi - register touchpad synchronously in probe
  Input: applespi - prefer asynchronous driver probing
  Input: applespi - fix use-after-free in applespi_remove()

 drivers/input/keyboard/applespi.c | 208 +++++++++++++++++++++++++-----
 1 file changed, 175 insertions(+), 33 deletions(-)

-- 
2.39.5

[PATCH v3 1/5] Input: applespi - use unified wait queue with timeouts for drain

[Shih-Yuan Lee]

Consolidate wait queues by renaming drain_complete to wait_queue and use
wait_event_lock_irq_timeout() with a 3-second timeout during read and write
drains to prevent indefinite lockups during driver unbind or PM transitions
if the hardware becomes unresponsive.

If a timeout occurs while a read or write transfer is still active, issue
a warning and fallback to waiting unconditionally until the transfer completion
callback clears the active flag. This guarantees that applespi_drain_writes()
and applespi_drain_reads() never return prematurely while transfers are in flight,
preventing use-after-free (UAF) and DMA memory corruption when devres frees
the driver context and DMA buffers during unbind.

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

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index b5ff71cd5a70..c501bf7e517e 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -417,7 +417,7 @@ struct applespi_data {
 
 	bool				suspended;
 	bool				drain;
-	wait_queue_head_t		drain_complete;
+	wait_queue_head_t		wait_queue;
 	bool				read_active;
 	bool				write_active;
 
@@ -677,7 +677,7 @@ static int applespi_setup_spi(struct applespi_data *applespi)
 		return sts;
 
 	spin_lock_init(&applespi->cmd_msg_lock);
-	init_waitqueue_head(&applespi->drain_complete);
+	init_waitqueue_head(&applespi->wait_queue);
 
 	return 0;
 }
@@ -725,7 +725,7 @@ static void applespi_msg_complete(struct applespi_data *applespi,
 		applespi->write_active = false;
 
 	if (applespi->drain && !applespi->write_active)
-		wake_up_all(&applespi->drain_complete);
+		wake_up_all(&applespi->wait_queue);
 
 	if (is_write_msg) {
 		applespi->cmd_msg_queued = 0;
@@ -1415,7 +1415,7 @@ static void applespi_got_data(struct applespi_data *applespi)
 			applespi->read_active = false;
 			applespi->write_active = false;
 
-			wake_up_all(&applespi->drain_complete);
+			wake_up_all(&applespi->wait_queue);
 		}
 
 		return;
@@ -1793,21 +1793,49 @@ static int applespi_probe(struct spi_device *spi)
 
 static void applespi_drain_writes(struct applespi_data *applespi)
 {
-	guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
+	unsigned long flags;
+	long ret;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
 
 	applespi->drain = true;
-	wait_event_lock_irq(applespi->drain_complete, !applespi->write_active,
-			    applespi->cmd_msg_lock);
+	ret = wait_event_lock_irq_timeout(applespi->wait_queue,
+					  !applespi->write_active,
+					  applespi->cmd_msg_lock,
+					  msecs_to_jiffies(3000));
+	if (!ret && applespi->write_active) {
+		dev_warn(&applespi->spi->dev,
+			 "Timed out waiting for write drain, waiting unconditionally\n");
+		wait_event_lock_irq(applespi->wait_queue,
+				    !applespi->write_active,
+				    applespi->cmd_msg_lock);
+	}
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
 }
 
 static void applespi_drain_reads(struct applespi_data *applespi)
 {
-	guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
+	unsigned long flags;
+	long ret;
 
-	wait_event_lock_irq(applespi->drain_complete, !applespi->read_active,
-			    applespi->cmd_msg_lock);
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	ret = wait_event_lock_irq_timeout(applespi->wait_queue,
+					  !applespi->read_active,
+					  applespi->cmd_msg_lock,
+					  msecs_to_jiffies(3000));
+	if (!ret && applespi->read_active) {
+		dev_warn(&applespi->spi->dev,
+			 "Timed out waiting for read drain, waiting unconditionally\n");
+		wait_event_lock_irq(applespi->wait_queue,
+				    !applespi->read_active,
+				    applespi->cmd_msg_lock);
+	}
 
 	applespi->suspended = true;
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
 }
 
 static void applespi_remove(struct spi_device *spi)
-- 
2.39.5

[PATCH v3 2/5] Input: applespi - track asynchronous SPI transfers in flight

[Shih-Yuan Lee]

The driver queues read and write packets asynchronously. When shutting
down, removing, or suspending, the driver must guarantee that no
asynchronous transfers remain in flight to prevent memory corruption or
use-after-free conditions.

Introduce a 'spi_complete' slot tracking array in struct applespi_data
to represent the two concurrent transfers (one for reads, one for
writes). Implement applespi_async_outstanding() and
applespi_async_complete() to track transfers under cmd_msg_lock.

Set applespi->cancel_spi = true during drain operations to reject new
asynchronous transfer requests with -ESHUTDOWN and activate completion
tracking.

In applespi_async_complete(), invoke the inner completion callback BEFORE
clearing the tracking slot and waking waiters on wait_queue. This ensures
that teardown threads waiting for in-flight transfers to drain cannot wake up
and free the driver structure before the completion callback finishes execution,
preventing use-after-free conditions.

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

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index c501bf7e517e..c9bbceaf0671 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -421,6 +421,12 @@ struct applespi_data {
 	bool				read_active;
 	bool				write_active;
 
+	struct applespi_complete_info {
+		void				(*complete)(void *context);
+		struct applespi_data		*applespi;
+	}				spi_complete[2];
+	bool				cancel_spi;
+
 	struct work_struct		work;
 	struct touchpad_info_protocol	rcvd_tp_info;
 
@@ -607,13 +613,73 @@ static void applespi_setup_write_txfrs(struct applespi_data *applespi)
 	spi_message_add_tail(st_t, msg);
 }
 
+static bool applespi_async_outstanding(struct applespi_data *applespi)
+{
+	return applespi->spi_complete[0].complete ||
+	       applespi->spi_complete[1].complete;
+}
+
+static void applespi_async_complete(void *context)
+{
+	struct applespi_complete_info *info = context;
+	struct applespi_data *applespi = info->applespi;
+	void (*complete)(void *context);
+	unsigned long flags;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+	complete = info->complete;
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+	if (complete)
+		complete(applespi);
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+	info->complete = NULL;
+
+	if (applespi->cancel_spi && !applespi_async_outstanding(applespi))
+		wake_up_all(&applespi->wait_queue);
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+}
+
 static int applespi_async(struct applespi_data *applespi,
 			  struct spi_message *message, void (*complete)(void *))
 {
-	message->complete = complete;
-	message->context = applespi;
+	struct applespi_complete_info *info;
+	int sts;
+
+	assert_spin_locked(&applespi->cmd_msg_lock);
+
+	if (applespi->cancel_spi) {
+		if (!applespi_async_outstanding(applespi))
+			wake_up_all(&applespi->wait_queue);
+		return -ESHUTDOWN;
+	}
+
+	/*
+	 * There can only be at most 2 spi requests in flight, one for "reads"
+	 * and one for "writes".
+	 */
+	if (!applespi->spi_complete[0].complete)
+		info = &applespi->spi_complete[0];
+	else if (!applespi->spi_complete[1].complete)
+		info = &applespi->spi_complete[1];
+	else {
+		dev_warn(&applespi->spi->dev, "Both SPI async slots in use\n");
+		return -EBUSY;
+	}
+
+	info->complete = complete;
+	info->applespi = applespi;
 
-	return spi_async(applespi->spi, message);
+	message->complete = applespi_async_complete;
+	message->context = info;
+
+	sts = spi_async(applespi->spi, message);
+	if (sts)
+		info->complete = NULL;
+
+	return sts;
 }
 
 static inline bool applespi_check_write_status(struct applespi_data *applespi,
@@ -1799,6 +1865,7 @@ static void applespi_drain_writes(struct applespi_data *applespi)
 	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
 
 	applespi->drain = true;
+	applespi->cancel_spi = true;
 	ret = wait_event_lock_irq_timeout(applespi->wait_queue,
 					  !applespi->write_active,
 					  applespi->cmd_msg_lock,
@@ -1821,6 +1888,8 @@ static void applespi_drain_reads(struct applespi_data *applespi)
 
 	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
 
+	applespi->cancel_spi = true;
+
 	ret = wait_event_lock_irq_timeout(applespi->wait_queue,
 					  !applespi->read_active,
 					  applespi->cmd_msg_lock,
-- 
2.39.5

[PATCH v3 3/5] Input: applespi - register touchpad synchronously in probe

[Shih-Yuan Lee]

Replace the asynchronous worker for touchpad registration with synchronous
registration during driver probe. Wait up to 3 seconds for the device info
packet and fallback to keyboard-only mode if a timeout occurs.

Prevent a kernel panic in debugfs by checking for a NULL touchpad_input_dev
in applespi_tp_dim_open() before dereferencing it, returning -ENODEV if the
device is operating in keyboard-only mode.

Protect rcvd_tp_info from data races by creating a local copy under
cmd_msg_lock before passing it to applespi_register_touchpad_device().

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

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index c9bbceaf0671..6db4eeca3060 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -427,7 +427,7 @@ struct applespi_data {
 	}				spi_complete[2];
 	bool				cancel_spi;
 
-	struct work_struct		work;
+	bool				have_tp_info;
 	struct touchpad_info_protocol	rcvd_tp_info;
 
 	struct dentry			*debugfs_root;
@@ -1030,6 +1030,9 @@ static int applespi_tp_dim_open(struct inode *inode, struct file *file)
 {
 	struct applespi_data *applespi = inode->i_private;
 
+	if (!applespi->touchpad_input_dev)
+		return -ENODEV;
+
 	file->private_data = applespi;
 
 	snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
@@ -1390,26 +1393,20 @@ applespi_register_touchpad_device(struct applespi_data *applespi,
 	return 0;
 }
 
-static void applespi_worker(struct work_struct *work)
-{
-	struct applespi_data *applespi =
-		container_of(work, struct applespi_data, work);
-
-	applespi_register_touchpad_device(applespi, &applespi->rcvd_tp_info);
-}
-
 static void applespi_handle_cmd_response(struct applespi_data *applespi,
 					 struct spi_packet *packet,
 					 struct message *message)
 {
+	unsigned long flags;
+
 	if (packet->device == PACKET_DEV_INFO &&
 	    le16_to_cpu(message->type) == 0x1020) {
-		/*
-		 * We're not allowed to sleep here, but registering an input
-		 * device can sleep.
-		 */
+		spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
 		applespi->rcvd_tp_info = message->tp_info;
-		schedule_work(&applespi->work);
+		applespi->have_tp_info = true;
+		spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+		wake_up_all(&applespi->wait_queue);
 		return;
 	}
 
@@ -1677,6 +1674,7 @@ static int applespi_probe(struct spi_device *spi)
 	acpi_handle spi_handle = ACPI_HANDLE(&spi->dev);
 	acpi_status acpi_sts;
 	int sts, i;
+	unsigned long flags;
 	unsigned long long gpe, usb_status;
 
 	/* check if the USB interface is present and enabled already */
@@ -1694,8 +1692,6 @@ static int applespi_probe(struct spi_device *spi)
 
 	applespi->spi = spi;
 
-	INIT_WORK(&applespi->work, applespi_worker);
-
 	/* store the driver data */
 	spi_set_drvdata(spi, applespi);
 
@@ -1823,6 +1819,25 @@ static int applespi_probe(struct spi_device *spi)
 	/* trigger touchpad setup */
 	applespi_init(applespi, false);
 
+	/* set up the touchpad as a separate input device if info is received */
+	sts = wait_event_timeout(applespi->wait_queue,
+				 READ_ONCE(applespi->have_tp_info),
+				 msecs_to_jiffies(3000));
+	if (!sts) {
+		dev_warn(&applespi->spi->dev,
+			 "Timed out waiting for touchpad info, continuing keyboard-only\n");
+	} else {
+		struct touchpad_info_protocol tp_info;
+
+		spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+		tp_info = applespi->rcvd_tp_info;
+		spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+		sts = applespi_register_touchpad_device(applespi, &tp_info);
+		if (sts)
+			goto cancel_spi;
+	}
+
 	/*
 	 * By default this device is not enabled for wakeup; but USB keyboards
 	 * generally are, so the expectation is that by default the keyboard
@@ -1855,6 +1870,19 @@ static int applespi_probe(struct spi_device *spi)
 			    &applespi_tp_dim_fops);
 
 	return 0;
+
+cancel_spi:
+	acpi_disable_gpe(NULL, applespi->gpe);
+	acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+	applespi->cancel_spi = true;
+	wait_event_lock_irq(applespi->wait_queue,
+			    !applespi_async_outstanding(applespi),
+			    applespi->cmd_msg_lock);
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+	return sts;
 }
 
 static void applespi_drain_writes(struct applespi_data *applespi)
-- 
2.39.5

[PATCH v3 4/5] Input: applespi - prefer asynchronous driver probing

[Shih-Yuan Lee]

Set probe_type to PROBE_PREFER_ASYNCHRONOUS to allow the driver core
to run applespi_probe() asynchronously. This improves system boot speeds by
avoiding blocking the main kernel thread during the 3-second touchpad
detection wait.

Additionally, clean up applespi_tp_dim_open() by simplifying product ID
retrieval and avoiding dereferencing touchpad_input_dev without a helper
variable.

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

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index 6db4eeca3060..a21e89f30387 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1029,6 +1029,7 @@ 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 = applespi->touchpad_input_dev;
 
 	if (!applespi->touchpad_input_dev)
 		return -ENODEV;
@@ -1037,7 +1038,7 @@ 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,
+		 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);
@@ -2044,6 +2045,7 @@ static struct spi_driver applespi_driver = {
 		.name			= "applespi",
 		.acpi_match_table	= applespi_acpi_match,
 		.pm			= pm_sleep_ptr(&applespi_pm_ops),
+		.probe_type		= PROBE_PREFER_ASYNCHRONOUS,
 	},
 	.probe		= applespi_probe,
 	.remove		= applespi_remove,
-- 
2.39.5

[PATCH v3 5/5] Input: applespi - fix use-after-free in applespi_remove()

[Shih-Yuan Lee]

Replace the separate read and write drain helpers with a single barrier using
cancel_spi and wait_event_lock_irq_timeout(). Wait for outstanding asynchronous
SPI operations to complete before tearing down the driver context.

Disable GPE and remove the GPE handler prior to setting cancel_spi = true.
This prevents level-triggered GPE interrupt storms where applespi_notify()
repeatedly attempts applespi_async() and gets rejected with -ESHUTDOWN while
the hardware interrupt line remains asserted.

If the 3-second wait times out while SPI transfers are still outstanding, issue
a warning and fallback to waiting unconditionally until all transfers complete,
guaranteeing that applespi_remove() never returns prematurely while transfers
are active.

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

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index a21e89f30387..602780842124 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1939,14 +1939,29 @@ 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);
+	unsigned long flags;
+	long ret;
 
-	applespi_drain_writes(applespi);
-
+	/* Disable GPE and remove handler first to prevent interrupt storm */
 	acpi_disable_gpe(NULL, applespi->gpe);
 	acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
 	device_wakeup_disable(&spi->dev);
 
-	applespi_drain_reads(applespi);
+	/* Prevent any new SPI transfers and wait for outstanding ones */
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+	applespi->cancel_spi = true;
+	ret = wait_event_lock_irq_timeout(applespi->wait_queue,
+					  !applespi_async_outstanding(applespi),
+					  applespi->cmd_msg_lock,
+					  msecs_to_jiffies(3000));
+	if (!ret && applespi_async_outstanding(applespi)) {
+		dev_warn(&applespi->spi->dev,
+			 "Timed out waiting for SPI transfers to drain, waiting unconditionally\n");
+		wait_event_lock_irq(applespi->wait_queue,
+				    !applespi_async_outstanding(applespi),
+				    applespi->cmd_msg_lock);
+	}
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
 
 	debugfs_remove_recursive(applespi->debugfs_root);
 }
-- 
2.39.5