[PATCH 1/2] serial: core: do not hold the port mutex while waiting for references

[Tapio Reijonen]

serial_core_remove_one_port() takes the port mutex and then waits for
the uart_port_ref() holders to drop their references. Since commit
8ca59f7baee7 ("serial: fix ioctl hangup race") uart_wait_until_sent()
takes the port mutex on every poll iteration to check for the hangup
before it touches the hardware, while holding its reference for the
whole wait. A tcdrain() in flight when the port is removed therefore
deadlocks: the removal holds the mutex and waits for the reference,
the drain holds the reference and waits for the mutex. Both tasks sit
uninterruptible, and the port is unusable until a reboot.

Reproduced by unbinding a UART driver while a process drains a few
kilobytes at 300 baud: the unbind blocks in wait_event() under the
mutex, the drain in mutex_lock(), and reading the driver's proc file
blocks on the same mutex.

Drop the mutex around the decrement and the wait. The decrement is an
atomic paired with uart_port_ref() and uart_port_deref(), which never
take the mutex; the clearing of the port pointer stays under it, where
uart_port_check() readers expect it. Between the hangup and this wait
the removal already runs without the mutex, so a mutex holder seeing
the released port is not a new state.
uart_wait_modem_status() takes a reference and then the mutex in the
same way, with a smaller window, and is covered by the same change.

Stable trees before the guard() conversion of this function need the
same change with explicit calls: mutex_unlock() before the
wait_event(), mutex_lock() around the store of the port pointer.
7.3.y has the guard() form and takes the patch with a three-way
merge.

Fixes: 8ca59f7baee7 ("serial: fix ioctl hangup race")
Cc: <stable@xxxxxxxxxxxxxxx> # 6.18.y and older need the open-coded form, see below
Signed-off-by: Tapio Reijonen <tapio.reijonen@xxxxxxxxxxx>
---
 drivers/tty/serial/serial_core.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c0deec0e6af41db10de15ccca4169c603d9cebf6..2e53d194ecfe853ade1ac6f33e047eb8ff11e283 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3266,10 +3266,17 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	uport->type = PORT_UNKNOWN;
 	uport->port_dev = NULL;
 
-	guard(mutex)(&port->mutex);
 	WARN_ON(atomic_dec_return(&state->refcount) < 0);
+
+	/*
+	 * A reference holder may need the port mutex to see the hangup
+	 * (uart_wait_until_sent() does), so wait for the references
+	 * without holding it.
+	 */
 	wait_event(state->remove_wait, !atomic_read(&state->refcount));
-	state->uart_port = NULL;
+
+	scoped_guard(mutex, &port->mutex)
+		state->uart_port = NULL;
 }
 
 static struct serial_ctrl_device *

-- 
2.47.3