Re: [PATCH] tty: serial: amba-pl011: cancel RS485 trigger hrtimers before removal
Greg KH <[email protected]> Thu, 30 Jul 2026 16:36:31 +0200
| Newsgroups | org.kernel.vger.linux-serial,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <2026073041-variably-jujitsu-411f@gregkh> |
On Tue, Jul 21, 2026 at 04:28:58AM +0000, Fan Wu wrote: > pl011_shutdown() calls pl011_rs485_tx_stop() when RS485 is enabled and > the transmit state machine is still active. That function may re-arm > the trigger_stop_tx hrtimer via hrtimer_start() when the TX FIFO is not > yet empty, or when delay_rts_after_send is configured, and the trigger > timers can equally be left armed from a prior pl011_rs485_tx_start(). > > On removal, uart_remove_one_port() can finish without draining the RS485 > hrtimers, and devm frees the uart_amba_port once pl011_remove() returns. > The existing teardown syncs do not help here: pl011_disable_interrupts() > only masks the UART IMSC register, pl011_dma_shutdown() cancels a > different (DMA rx) timer_list, and free_irq() plus serial_core's > synchronize_irq() only drain the UART interrupt handler, not the hrtimer > softirq callbacks. Either trigger callback can therefore recover uap > from its embedded hrtimer and access uap->port after the free. > > Cancel both RS485 trigger hrtimers in pl011_shutdown(), after the > re-arming pl011_rs485_tx_stop() call and before free_irq(), and again > in pl011_remove() after uart_remove_one_port(). The remove-side > calls cover paths such as suspend followed by unbind, where serial > core need not invoke the driver's shutdown callback. The > callbacks pl011_trigger_start_tx and pl011_trigger_stop_tx return > HRTIMER_NORESTART, so hrtimer_cancel() fully drains any pending and > concurrently running callback without a self-restart loop. As > pl011_trigger_start_tx() may re-enable the TX interrupt via > pl011_start_tx(), call pl011_disable_interrupts() again after the > cancels to re-mask IMSC before free_irq(). The cancel site does not > hold uart_port_lock across the cancel > (pl011_disable_interrupts() already released it) and the callbacks take > that lock internally, so there is no self-deadlock. This mirrors the > RS485 teardown pattern in 8250_port.c (serial8250_em485_destroy), > which cancels both hrtimers before the embedding object is freed. > > This issue was found by an in-house static analysis tool. Yes, but a LLM obviously wrote the text here. Please shorten it up a LOT and make it more concise, as-is, it's impossible to read this wall-of-text... thanks, greg k-h