[PATCH v2] Input: serport - defer write wakeups to a worker

[PATCH v2] Input: serport - defer write wakeups to a worker

[James Hilliard]

UART drivers can invoke the line discipline's write_wakeup() callback with
their port lock held. serport forwards the notification directly to the
serio driver. I-Force's callback can transmit a queued packet through
serio_write(), which calls back into uart_write() and tries to acquire the
same port lock. A notification after I-Force queues a packet but before it
sets IFORCE_XMIT_RUNNING can therefore deadlock.

Defer the serio notification to a work item, as required for transmitting
from a line discipline's write-wakeup path. Initialize the work disabled
and enable it, along with TTY_DO_WRITE_WAKEUP, only when a serio driver
with a write_wakeup() callback opens the port.

Clear TTY_DO_WRITE_WAKEUP and disable and drain the work when that driver
closes the port, before the serio driver and port can be torn down. This
allows the worker to call the driver without holding serport->lock,
breaking the lock ordering cycle with input event callbacks that write
to the UART.

Also clear TTY_DO_WRITE_WAKEUP and disable and drain the work on hangup,
before waking the reader to unregister the serio port. The additional
disable keeps the work disabled even if a driver opens the port while
hangup is in progress.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes in v2:
- Follow Dmitry's lifecycle suggestion: initialize the work disabled and
  enable notifications only for a serio driver with a write_wakeup callback.
- Disable and drain the work in serio close before the driver is detached,
  allowing the worker to call the driver without holding serport->lock.
- Disable and drain the work in hangup before waking the reader, keeping
  it disabled even if a driver probe races with hangup.
- Link to v1: https://patch.msgid.link/20261023-submit-serport-write-wakeup-v1-v1-1-74e7311638a2@gmail.com
---
 drivers/input/serio/serport.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index cabe3876c168..e522aa7f0fc2 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -18,6 +18,7 @@
 #include <linux/serio.h>
 #include <linux/tty.h>
 #include <linux/compat.h>
+#include <linux/workqueue.h>
 
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@ucw.cz>");
 MODULE_DESCRIPTION("Input device TTY line discipline");
@@ -33,6 +34,7 @@ struct serport {
 	wait_queue_head_t wait;
 	struct serio *serio;
 	struct serio_device_id id;
+	struct work_struct write_wakeup;
 	spinlock_t lock;
 	unsigned long flags;
 };
@@ -54,6 +56,11 @@ static int serport_serio_open(struct serio *serio)
 	guard(spinlock_irqsave)(&serport->lock);
 	set_bit(SERPORT_ACTIVE, &serport->flags);
 
+	if (serio->drv->write_wakeup) {
+		enable_work(&serport->write_wakeup);
+		set_bit(TTY_DO_WRITE_WAKEUP, &serport->tty->flags);
+	}
+
 	return 0;
 }
 
@@ -62,8 +69,20 @@ static void serport_serio_close(struct serio *serio)
 {
 	struct serport *serport = serio->port_data;
 
-	guard(spinlock_irqsave)(&serport->lock);
-	clear_bit(SERPORT_ACTIVE, &serport->flags);
+	scoped_guard(spinlock_irqsave, &serport->lock)
+		clear_bit(SERPORT_ACTIVE, &serport->flags);
+
+	if (serio->drv->write_wakeup) {
+		clear_bit(TTY_DO_WRITE_WAKEUP, &serport->tty->flags);
+		disable_work_sync(&serport->write_wakeup);
+	}
+}
+
+static void serport_write_wakeup(struct work_struct *work)
+{
+	struct serport *serport = container_of(work, struct serport, write_wakeup);
+
+	serio_drv_write_wakeup(serport->serio);
 }
 
 /*
@@ -85,10 +104,11 @@ static int serport_ldisc_open(struct tty_struct *tty)
 	serport->tty = tty;
 	spin_lock_init(&serport->lock);
 	init_waitqueue_head(&serport->wait);
+	INIT_WORK(&serport->write_wakeup, serport_write_wakeup);
+	disable_work(&serport->write_wakeup);
 
 	tty->disc_data = serport;
 	tty->receive_room = 256;
-	set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 
 	return 0;
 }
@@ -239,6 +259,10 @@ static void serport_ldisc_hangup(struct tty_struct *tty)
 {
 	struct serport *serport = tty->disc_data;
 
+	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
+	/* Keep work disabled even if a driver opens the port during hangup. */
+	disable_work_sync(&serport->write_wakeup);
+
 	scoped_guard(spinlock_irqsave, &serport->lock)
 		set_bit(SERPORT_DEAD, &serport->flags);
 
@@ -249,10 +273,8 @@ static void serport_ldisc_write_wakeup(struct tty_struct * tty)
 {
 	struct serport *serport = tty->disc_data;
 
-	guard(spinlock_irqsave)(&serport->lock);
-
-	if (test_bit(SERPORT_ACTIVE, &serport->flags))
-		serio_drv_write_wakeup(serport->serio);
+	/* A serio driver may write to the TTY, so defer the callback. */
+	schedule_work(&serport->write_wakeup);
 }
 
 /*

---
base-commit: f57202d55333f750a18e21c3b3f35f48ef14a61e
change-id: 20261023-submit-serport-write-wakeup-v1-1bdd1006fafa

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

Re: [PATCH v2] Input: serport - defer write wakeups to a worker

[sashiko-bot]

> UART drivers can invoke the line discipline's write_wakeup() callback with
> their port lock held. serport forwards the notification directly to the
> serio driver. I-Force's callback can transmit a queued packet through
> serio_write(), which calls back into uart_write() and tries to acquire the
> same port lock. A notification after I-Force queues a packet but before it
> sets IFORCE_XMIT_RUNNING can therefore deadlock.
> 
> Defer the serio notification to a work item, as required for transmitting
> from a line discipline's write-wakeup path. Initialize the work disabled
> and enable it, along with TTY_DO_WRITE_WAKEUP, only when a serio driver
> with a write_wakeup() callback opens the port.
> 
> Clear TTY_DO_WRITE_WAKEUP and disable and drain the work when that driver
> closes the port, before the serio driver and port can be torn down. This
> allows the worker to call the driver without holding serport->lock,
> breaking the lock ordering cycle with input event callbacks that write
> to the UART.
> 
> Also clear TTY_DO_WRITE_WAKEUP and disable and drain the work on hangup,
> before waking the reader to unregister the serio port. The additional
> disable keeps the work disabled even if a driver opens the port while
> hangup is in progress.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-submit-serport-write-wakeup-v1-v2-1-c05d96259985@gmail.com?part=1

Re: [PATCH v2] Input: serport - defer write wakeups to a worker

[Dmitry Torokhov]

Hi James,

On Wed, Oct 28, 2026 at 01:14:19AM -0600, James Hilliard wrote:
> @@ -54,6 +56,11 @@ static int serport_serio_open(struct serio *serio)
>  	guard(spinlock_irqsave)(&serport->lock);

I changed this to scoped_guard() and applied.

>  	set_bit(SERPORT_ACTIVE, &serport->flags);
>  
> +	if (serio->drv->write_wakeup) {
> +		enable_work(&serport->write_wakeup);
> +		set_bit(TTY_DO_WRITE_WAKEUP, &serport->tty->flags);
> +	}
> +
>  	return 0;
>  }

Thanks.

-- 
Dmitry