* [PATCH] tty: serial: samsung: use dmaengine_terminate_sync() in release_dma @ 2026-10-30 16:23 ` Samarth Narang 2026-10-30 16:32 ` sashiko-bot 2026-10-31 0:02 ` Marek Szyprowski 0 siblings, 2 replies; 3+ messages in thread From: Samarth Narang @ 2026-10-30 16:23 UTC (permalink / raw) To: Krzysztof Kozlowski, Peter Griffin, Greg Kroah-Hartman, Jiri Slaby Cc: Alim Akhtar, linux-samsung-soc, linux-serial, linux-arm-kernel, linux-kernel, Samarth Narang s3c24xx_serial_release_dma() stops the RX and TX DMA channels using the deprecated dmaengine_terminate_all() before unmapping their buffers and freeing the RX buffer. Since this does not guarantee that DMA transfers or their completion callbacks have finished, the controller may continue accessing memory after it has been unmapped or returned to the allocator, particularly when the port is shut down while RX DMA is active. Replace dmaengine_terminate_all() with dmaengine_terminate_sync() to ensure that outstanding DMA activity is terminated before the buffers are released. Both callers of s3c24xx_serial_release_dma() run in process context, where synchronous termination is permitted. Fixes: 62c37eedb74c ("serial: samsung: add dma reqest/release functions") Assisted-by: LLM Signed-off-by: Samarth Narang <samarth.colleges@gmail.com> --- This issue was identified by a research static analyzer for DMA lifecycle ordering and has not been validated on hardware. The patch has only been compile-tested on arm64 with W=1. On Exynos systems, the UART uses PL330, which does not implement a device_synchronize callback, so this change follows the DMAengine API contract without independently establishing that all hardware activity has stopped. The PL330 termination behavior is being raised separately with the DMAengine maintainers. Two related questions outside the scope of this patch: - s3c24xx_serial_rx_dma_complete() unconditionally re-arms RX DMA, potentially racing with release_dma() and submitting another descriptor after termination. Should shutdown prevent further submissions before terminating the channel? - s3c24xx_serial_stop_rx() calls dmaengine_pause(dma->tx_chan) before checking the RX channel's residue. Should this instead use dma->rx_chan? drivers/tty/serial/samsung_tty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index 640f85e2d..4eeef149b 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -1145,7 +1145,7 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p) struct s3c24xx_uart_dma *dma = p->dma; if (dma->rx_chan) { - dmaengine_terminate_all(dma->rx_chan); + dmaengine_terminate_sync(dma->rx_chan); dma_unmap_single(dmaengine_get_dma_device(dma->rx_chan), dma->rx_addr, dma->rx_size, DMA_FROM_DEVICE); kfree(dma->rx_buf); @@ -1154,7 +1154,7 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p) } if (dma->tx_chan) { - dmaengine_terminate_all(dma->tx_chan); + dmaengine_terminate_sync(dma->tx_chan); dma_unmap_single(dmaengine_get_dma_device(dma->tx_chan), dma->tx_addr, UART_XMIT_SIZE, DMA_TO_DEVICE); dma_release_channel(dma->tx_chan); base-commit: 83f32c773255720a661aa45d83e36278135f23f5 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] tty: serial: samsung: use dmaengine_terminate_sync() in release_dma 2026-10-30 16:23 ` [PATCH] tty: serial: samsung: use dmaengine_terminate_sync() in release_dma Samarth Narang @ 2026-10-30 16:32 ` sashiko-bot 2026-10-31 0:02 ` Marek Szyprowski 1 sibling, 0 replies; 3+ messages in thread From: sashiko-bot @ 2026-10-30 16:32 UTC (permalink / raw) To: Samarth Narang; +Cc: linux-serial > s3c24xx_serial_release_dma() stops the RX and TX DMA channels using the > deprecated dmaengine_terminate_all() before unmapping their buffers and > freeing the RX buffer. Since this does not guarantee that DMA transfers > or their completion callbacks have finished, the controller may continue > accessing memory after it has been unmapped or returned to the > allocator, particularly when the port is shut down while RX DMA is > active. > > Replace dmaengine_terminate_all() with dmaengine_terminate_sync() to > ensure that outstanding DMA activity is terminated before the buffers > are released. Both callers of s3c24xx_serial_release_dma() run in > process context, where synchronous termination is permitted. > > Fixes: 62c37eedb74c ("serial: samsung: add dma reqest/release functions") > Assisted-by: LLM > Signed-off-by: Samarth Narang <samarth.colleges@gmail.com> Sashiko has reviewed this patch and found no issues. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261030162334.3000737-1-samarth.colleges@gmail.com?part=1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tty: serial: samsung: use dmaengine_terminate_sync() in release_dma 2026-10-30 16:23 ` [PATCH] tty: serial: samsung: use dmaengine_terminate_sync() in release_dma Samarth Narang 2026-10-30 16:32 ` sashiko-bot @ 2026-10-31 0:02 ` Marek Szyprowski 1 sibling, 0 replies; 3+ messages in thread From: Marek Szyprowski @ 2026-10-31 0:02 UTC (permalink / raw) To: Samarth Narang, Krzysztof Kozlowski, Peter Griffin, Greg Kroah-Hartman, Jiri Slaby Cc: Alim Akhtar, linux-samsung-soc, linux-serial, linux-arm-kernel, linux-kernel On 30.10.2026 17:23, Samarth Narang wrote: > s3c24xx_serial_release_dma() stops the RX and TX DMA channels using the > deprecated dmaengine_terminate_all() before unmapping their buffers and > freeing the RX buffer. Since this does not guarantee that DMA transfers > or their completion callbacks have finished, the controller may continue > accessing memory after it has been unmapped or returned to the > allocator, particularly when the port is shut down while RX DMA is > active. > > Replace dmaengine_terminate_all() with dmaengine_terminate_sync() to > ensure that outstanding DMA activity is terminated before the buffers > are released. Both callers of s3c24xx_serial_release_dma() run in > process context, where synchronous termination is permitted. > > Fixes: 62c37eedb74c ("serial: samsung: add dma reqest/release functions") > Assisted-by: LLM > Signed-off-by: Samarth Narang <samarth.colleges@gmail.com> Reviewed-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > This issue was identified by a research static analyzer for DMA > lifecycle ordering and has not been validated on hardware. The patch has > only been compile-tested on arm64 with W=1. > > On Exynos systems, the UART uses PL330, which does not implement a > device_synchronize callback, so this change follows the DMAengine API > contract without independently establishing that all hardware activity > has stopped. The PL330 termination behavior is being raised separately > with the DMAengine maintainers. > > Two related questions outside the scope of this patch: > > - s3c24xx_serial_rx_dma_complete() unconditionally re-arms RX DMA, > potentially racing with release_dma() and submitting another > descriptor after termination. Should shutdown prevent further > submissions before terminating the channel? Yes, this need to be fixed. > - s3c24xx_serial_stop_rx() calls dmaengine_pause(dma->tx_chan) before > checking the RX channel's residue. Should this instead use > dma->rx_chan? Right, definitely that's a bug in current code. > > drivers/tty/serial/samsung_tty.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index 640f85e2d..4eeef149b 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -1145,7 +1145,7 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p) > struct s3c24xx_uart_dma *dma = p->dma; > > if (dma->rx_chan) { > - dmaengine_terminate_all(dma->rx_chan); > + dmaengine_terminate_sync(dma->rx_chan); > dma_unmap_single(dmaengine_get_dma_device(dma->rx_chan), dma->rx_addr, > dma->rx_size, DMA_FROM_DEVICE); > kfree(dma->rx_buf); > @@ -1154,7 +1154,7 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p) > } > > if (dma->tx_chan) { > - dmaengine_terminate_all(dma->tx_chan); > + dmaengine_terminate_sync(dma->tx_chan); > dma_unmap_single(dmaengine_get_dma_device(dma->tx_chan), dma->tx_addr, > UART_XMIT_SIZE, DMA_TO_DEVICE); > dma_release_channel(dma->tx_chan); > > base-commit: 83f32c773255720a661aa45d83e36278135f23f5 > > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-31 0:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20261030162739eucas1p18c5a6e164b995c7e15adf4d989577e4c@eucas1p1.samsung.com>
2026-10-30 16:23 ` [PATCH] tty: serial: samsung: use dmaengine_terminate_sync() in release_dma Samarth Narang
2026-10-30 16:32 ` sashiko-bot
2026-10-31 0:02 ` Marek Szyprowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.