Re: [BUG] crypto: starfive - DMA left running after timeout while buffers are unmapped

[Samarth Narang]

Hello,

The two addresses listed under STARFIVE CRYPTO DRIVER in MAINTAINERS
(Jia Jie Ho and William Qiu) both bounced this report with "550 5.4.1
Recipient address rejected".  I have added some more folks on this
thread, who seem to be active in the STARFIVE ecosystem who perhaps
could weigh in on the report quoted below.

Thanks,
Samarth

On Fri, Oct 30, 2026 at 9:02 PM Samarth Narang
<samarth.colleges@xxxxxxxxx> wrote:
>
> Hi,
>
> I found two DMA cleanup issues in the StarFive JH7110 AES and hash
> drivers. On timeout, or when preparation of a subsequent descriptor
> fails, the drivers can return an error and unmap their scatterlists
> without terminating previously issued DMA descriptors.
>
> Line numbers refer to mainline at 83f32c773255. I could not find any
> dmaengine_terminate_*() calls under drivers/crypto/starfive/.
>
> 1. AES (jh7110-aes.c)
>
> starfive_aes_dma_xfer() submits and issues out_desc on cryp->rx (lines
> 471-472), followed by in_desc on cryp->tx (lines 481-482). It then waits
> up to one second for the RX descriptor's completion callback (line 484).
>
> If the wait times out, the function clears alg_cr and returns
> -ETIMEDOUT. The caller, starfive_aes_map_sg(), subsequently unmaps the
> source and destination scatterlists (lines 520, 540-541) and propagates
> the error.
>
> Nothing in this error path explicitly terminates either DMA channel. The
> descriptors may therefore retain the ability to access their mappings
> after those mappings have been released.
>
> There is also a descriptor-preparation failure path. If
> dmaengine_prep_slave_sg() fails for in_desc (line 474), the function
> returns -EINVAL even though out_desc has already been issued on
> cryp->rx. The destination scatterlist is subsequently unmapped at line
> 541.
>
> Since the RX descriptor transfers data from the device to memory, any
> later execution of that descriptor could write to storage whose DMA
> mapping has already been released.
>
> 2. Hash (jh7110-hash.c)
>
> starfive_hash_dma_xfer() submits and issues in_desc on cryp->tx (lines
> 150-151), then waits up to one second for completion (line 153).
>
> On timeout, the function clears alg_cr and returns an error.
> starfive_hash_one_request() subsequently unmaps the source scatterlist
> at line 235.
>
> The TX descriptor has not been explicitly terminated before the unmap,
> so it may retain access to the old source mapping.
>
> DMA provider and possible consequences
>
> On JH7110, the AES and hash DMA channels use the sdma controller,
> associated with the PL080 DMAengine implementation through jh7110.dtsi.
>
> The transfers are flow-controlled by the crypto hardware. I could not
> establish from the driver source whether clearing alg_cr fully
> suppresses further DMA requests, drains requests already accepted by the
> DMA controller, or retires the corresponding descriptors.
>
> Even if clearing the register prevents immediate further transfers, that
> does not by itself establish that the DMAengine descriptor has been
> retired. If the controller can resume the outstanding descriptor when
> requests become available again, a later crypto operation could cause
> accesses using the previous request's mapping.
>
> Depending on the DMA mapping configuration and subsequent memory reuse,
> such accesses could read from or write to storage belonging to another
> operation.
>
> Proposed fix
>
> I think the error paths should explicitly terminate the outstanding DMA
> operations before returning control to the callers that unmap the
> scatterlists.
>
> Specifically:
>
> In starfive_aes_dma_xfer(), synchronously terminate both RX and TX
> channels after a timeout, and terminate the already-issued RX channel if
> preparation or submission of the TX descriptor fails.
>
> In starfive_hash_dma_xfer(), synchronously terminate the TX channel on
> timeout.
>
> Both functions execute in the crypto engine worker context and already
> use wait_for_completion_timeout(), so a synchronous termination call
> appears appropriate. The cleanup would need to preserve the original
> error and account for the outcome of termination.
>
> Would dmaengine_terminate_sync() be the preferred approach here, or is
> there a crypto-controller-specific shutdown sequence that must precede
> DMA termination?
>
> I can prepare and compile-test a patch if this direction is acceptable.
>
> These cases were identified by a research static analyzer for DMA
> lifecycle ordering and subsequently reviewed against the source. I have
> not reproduced a timeout or late DMA access on JH7110 hardware. The
> reported issue is the absence of established DMA retirement before
> unmapping, rather than an experimentally demonstrated memory corruption.
>
> Thanks,
> Samarth