[PATCH 2/2] serial: core: do not touch the port after dropping the last reference

[Tapio Reijonen]

uart_port_deref() decrements the reference count through uport->state
and then reads uport->state again for the wake_up(). The last
decrement is what lets serial_core_remove_one_port() return, and the
driver may free its struct uart_port right after that. A reference
holder that drops the last reference can therefore read the freed port
on its way to the wake_up().

Read the state pointer once, before the decrement. The uart_state
belongs to the uart_driver and lives until uart_unregister_driver(),
so the wake_up() target outlives the port. Drivers that unregister the
uart_driver in the same remove() are a separate matter and not changed
here.

Found by review of the removal path while fixing the deadlock between
port removal and a blocked tcdrain(); not reproduced.

Fixes: 9ed19428a51d ("serial: core: Prevent unsafe uart port access, part 2")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Tapio Reijonen <tapio.reijonen@xxxxxxxxxxx>
---
 drivers/tty/serial/serial_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 2e53d194ecfe853ade1ac6f33e047eb8ff11e283..c3d21acbd606eb3ea2909736a01a66f204141be1 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -72,8 +72,10 @@ static inline struct uart_port *uart_port_ref(struct uart_state *state)
 
 static inline void uart_port_deref(struct uart_port *uport)
 {
-	if (atomic_dec_and_test(&uport->state->refcount))
-		wake_up(&uport->state->remove_wait);
+	struct uart_state *state = uport->state;
+
+	if (atomic_dec_and_test(&state->refcount))
+		wake_up(&state->remove_wait);
 }
 
 static inline struct uart_port *uart_port_ref_lock(struct uart_state *state, unsigned long *flags)

-- 
2.47.3