On Fri, Oct 30, 2026 at 11:36 AM Raul E Rangel [off-list ref] wrote:
On Thu, Oct 29, 2026 at 6:35 PM James Hilliard [off-list ref] wrote:
quoted
That is the current behavior. Returning from shutdown allows serial
core to free the TX buffer and removal to release the callback
context. The retry loop prevents those releases, but a permanent
termination failure blocks teardown indefinitely
...
quoted
Retaining the buffers and channels does not preserve the complete
callback context.
The callbacks carry a uart_8250_port pointer. For DW, the DMA state is
embedded in devm-managed private data, which can be freed after
unbind. The registered 8250 port slot can also be reused. Retaining
the DMA channel does not prevent either operation.
...
quoted
Clearing up->dma does not stop hardware accesses. After unsuccessful
termination, the channel may still access the UART FIFO, so reopening
with PIO can introduce concurrent FIFO access.
A hardware-specific stop/reset sequence could establish quiescence,
but its implementation and effects on other channels depend on the
controller. If the channel cannot be stopped, quarantine also needs
to preserve callback-context lifetimes and prevent unsafe port reuse.
...
quoted
The current callbacks fetch p->dma before acquiring the port lock.
A callback can therefore retain the old non-NULL pointer across the
assignment of NULL. Checking that cached pointer does not protect
its backing allocation from being freed.
When termination returns an error, dmaengine_terminate_sync() also
returns without calling dmaengine_synchronize(), so callback draining
has not been established.
Those are all good points. We added port reopen and a `NoConcurrentFifoAccess`
invariant to `Serial8250TxDmaV8.tla`
(`Serial8250TxDmaV8_QuarantinePioReopenHazard.cfg`), and TLC immediately
confirmed your point on FIFO contention: clearing `up->dma = NULL` and
falling back to PIO when reopening after a failed `dmaengine_terminate_sync()`
violates `NoConcurrentFifoAccess` in 7 states because the un-terminated DMA
descriptor can still access the hardware UART FIFO concurrently with PIO.
We can satisfy all three constraints while still preventing normal `close()`,
`hangup()`, and system `suspend()` from hanging forever in
`TASK_UNINTERRUPTIBLE` under `port->mutex` across the three lifecycle edges:
Keeping the DMA state intact and refusing PIO reuse addresses the earlier
reopen hazard. However, implementing quarantine safely requires changes
beyond retaining the buffers and rejecting startup.
For example, serial8250_do_shutdown() clears the FIFOs and reads UART_RX
after serial8250_release_dma() returns. Those operations also need to be
excluded while the retained channel may still access the UART. Close and
suspend subsequently enter power-down paths, so retaining the allocations
alone does not establish that either transition is safe.
There is also a serial-core exception to the proposed reopen contract:
startup errors permit an administrative open for CAP_SYS_ADMIN. That
allows reconfiguration paths which can release port resources. Returning
-EIO from the driver startup callback therefore does not by itself
enforce quarantine.
1. **On `close()` / `hangup()` / `uart_suspend_port()` (`serial8250_release_dma()` & `uart_free_xmit_buf()`)**:
- Bound the retry loop in `serial8250_terminate_dma()` (with a short
backoff rather than an unbounded `msleep(100)`, guarding
`if (dma->rxchan)` / `if (dma->txchan)`) and release whichever
channel(s) terminated cleanly (`rx_ret == 0` or `tx_ret == 0`).
- If `rx_ret != 0` or `tx_ret != 0`, **keep `up->dma` intact** (do not set
`up->dma = NULL`, including in `8250_omap.c:omap_8250_shutdown()`), keep
`dma->shutting_down = true`, and retain the failed channel and its
buffer (`dma->rxchan` / `dma->rx_buf` when `rx_ret != 0`; `dma->txchan`
and mapped `dma->tx_addr` when `tx_ret != 0`).
- When `tx_ret != 0`, explicitly call `uart_xmit_block(&p->port)` under
`p->port.lock` (`state->xmit_blocked = true`, since `xmit_blocked` is
otherwise only set when `uart_flush_buffer()` ran prior to `close()`),
and in `serial_core.c:uart_free_xmit_buf()`, return early under
`uport->lock` if `state->xmit_blocked` is set (without clearing
`port->xmit_buf = NULL`, resetting `port->xmit_fifo`, or calling
`kfree(xmit_buf)`).
- Because `up->dma` is not cleared and `struct uart_8250_dma` /
`struct uart_8250_port` remain allocated on the bound device during
`close()`, `hangup()`, and `suspend()`, any callback that was not
drained by `dmaengine_terminate_sync()` safely loads a valid `p->dma`,
observes `dma->shutting_down == true` under `port->lock`, and returns
without UAF—while `close()`, `hangup()`, and system `suspend()` return
cleanly without hanging in `D` state or triggering `hung_task_panic`.
2. **On subsequent `open()` (`serial8250_do_startup()` & `omap_8250_startup()`)**:
- At the very start of `serial8250_do_startup()` (`8250_port.c:2356`) and
`omap_8250_startup()` (`8250_omap.c:724`)—after acquiring runtime PM,
and **before** clearing `up->dma->shutting_down = false`, touching
`UART_FCR` / `UART_RX` / `UART_TX`, or setting up IRQs/timers—if
`up->dma && (up->dma->rxchan || up->dma->txchan)` is still non-NULL from
a prior failed termination, retry `serial8250_release_dma(up)` first:
- If `up->dma->rxchan || up->dma->txchan` is **still** non-NULL
(termination still failed), immediately return `-EIO` and **fail
`startup()`** (do *not* clear `up->dma = NULL` and do *not* fall back
to PIO while the wedged DMA channel may still touch the UART FIFO).
- If termination succeeded on retry (`rxchan == NULL && txchan == NULL`,
which also unblocked `state->xmit_blocked`), reset `xmit_fifo` and
proceed with normal startup and `serial8250_request_dma(up)`.
3. **On driver unbind (`serial8250_unregister_port()` / `serial_core_remove_one_port()`)**:
- Driver removal is the *only* point where `devm`-allocated
`struct uart_8250_dma` and the `serial8250_ports[]` slot are about to be
freed/reused (note also that `serial_core_remove_one_port()` at
`serial_core.c:3336` only calls `uart_shutdown()` when
`tty_port_initialized(port)` is true, so if the port was already closed
with a quarantined channel, unbind must still drain/release it).
- Restricting blocking termination wait to driver unbind (when a channel is
still active/quarantined) prevents `close()`, `hangup()`, and system
`suspend()` from ever hanging the kernel in `msleep(100)`.
This looks like a useful direction, but it introduces a broader lifecycle
contract across serial core, the individual drivers and power management.
In particular, suspend may need to fail until the hardware is quiescent,
rather than succeed with a quarantined channel.
The current retry loop still allows permanent termination failure to
block teardown; changing the backoff does not resolve that limitation.
Would it make sense to address quarantine separately, with that
limitation documented explicitly here? I would prefer not to fold this
larger redesign into the cancellation series before those paths have
been worked through.
We verified this quarantine + `-EIO` reopen contract in
`Serial8250TxDmaV8_TerminateRetryHang_Fixed.cfg` (`AllowReopen = TRUE`,
`ReopenFallbackToPio = FALSE`), which passes all safety and liveness
properties (`NoUsersAtFree`, `BoundedTerminateRetries`,
`NoConcurrentFifoAccess`, `QuarantineImpliesXmitBlocked`, `ShutdownProgress`)
with 0 errors:
https://github.com/ismell/linux-8250-dma-tla
Thanks,
Raul