On Thu, Oct 29, 2026 at 01:37:02AM -0600, James Hilliard wrote:
serial8250_release_dma() ignores the return from dmaengine_terminate_sync()
before freeing the coherent RX buffer, unmapping the TX FIFO and releasing
the channels. If the provider fails to stop a channel, the DMAengine
wrapper returns the error without synchronizing its callbacks. Hardware
or a detached callback can then access the released storage and state.
Checking the error and returning early would not preserve their lifetime:
serial core frees the TX FIFO after the driver's shutdown callback returns,
and port removal can then release the callback context as well.
Require successful synchronous termination of both channels before
releasing any DMA resources. Retry failed channels after a sleep outside
the port lock, leaving channels that have already stopped and drained
alone. Keep the shutdown gate and storage reservation in place throughout
the retries.
A permanently failed DMA engine blocks close or removal. This deliberately
retains the buffers, mappings, channels and callback context until the
provider can establish that they are no longer in use.
...
quoted hunk ↗ jump to hunk
+ while (rx_ret || tx_ret) {
+ dev_err_ratelimited(p->port.dev,
+ "DMA termination failed: RX %d, TX %d; retrying\n",
+ rx_ret, tx_ret);
+ msleep(100);
Huh?!
quoted hunk ↗ jump to hunk
+ if (rx_ret)
+ rx_ret = dmaengine_terminate_sync(dma->rxchan);
+ if (tx_ret)
+ tx_ret = dmaengine_terminate_sync(dma->txchan);
+ }
This is unacceptable. Sleeping is okay, but value should really match the
baudspeed and other parameters of the transfer.
--
With Best Regards,
Andy Shevchenko