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

[Johan Hovold <johan@kernel.org>]

On Thu, Nov 12, 2026 at 04:37:28AM +0000, Tapio Reijonen wrote:
> 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@vger.kernel.org
> Reviewed-by: Johan Hovold <johan@kernel.org>
> Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
 
>  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;

Hmm. Don't we also need a READ_ONCE() here to prevent the compiler from
doing something silly?

> +
> +	if (atomic_dec_and_test(&state->refcount))
> +		wake_up(&state->remove_wait);
>  }

Johan