[PATCHv2] crypto: amcc - fix missing DMA memory barriers in descriptor handling

[PATCHv2] crypto: amcc - fix missing DMA memory barriers in descriptor handling

[Rosen Penev]

Fix two memory ordering bugs in the AMCC crypto driver:

1. In crypto4xx_bh_tasklet_cb(), add a dma_rmb() between reading the
   PD_CTL_PE_DONE bit (with READ_ONCE) and reading the descriptor and DMA
   buffer data in crypto4xx_pd_done().  Without it, the CPU on a
   weakly-ordered architecture could read stale descriptor data before the
   hardware's writes are globally visible.

2. In crypto4xx_build_pd(), add a dma_wmb() before writing
   PD_CTL_HOST_READY to ensure all descriptor and SA data is visible to
   the device before the ownership handover bit.  Also fix the descriptor
   field ordering: pd_ctl_len.w must be written before pd_ctl.w (which
   contains HOST_READY), not after, to prevent the hardware from fetching
   uninitialized length data.

Fixes: f6c48b76daa6 (crypto: amcc - Add crypto4xx-aead cryptographic
  AEAD accelerator driver)
Cc: stable@vger.kernel.org
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: add descriptions for write barriers.
 drivers/crypto/amcc/crypto4xx_core.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c
index fd010bfb7020..4b0ce165c198 100644
--- a/drivers/crypto/amcc/crypto4xx_core.c
+++ b/drivers/crypto/amcc/crypto4xx_core.c
@@ -876,11 +876,17 @@ int crypto4xx_build_pd(struct crypto_async_request *req,
 		}
 	}
 
+	pd->pd_ctl_len.w = 0x00400000 | (assoclen + datalen);
+	pd_uinfo->state = PD_ENTRY_INUSE | (is_busy ? PD_ENTRY_BUSY : 0);
+
+	/* make the pd_ctl_len and pd_uinfo->state writes above visible to
+	 * the device before the HOST_READY handover write below, so the
+	 * device never fetches a descriptor with stale length/state bits
+	 */
+	dma_wmb();
 	pd->pd_ctl.w = PD_CTL_HOST_READY |
 		((crypto_tfm_alg_type(req->tfm) == CRYPTO_ALG_TYPE_AEAD) ?
 			PD_CTL_HASH_FINAL : 0);
-	pd->pd_ctl_len.w = 0x00400000 | (assoclen + datalen);
-	pd_uinfo->state = PD_ENTRY_INUSE | (is_busy ? PD_ENTRY_BUSY : 0);
 
 	wmb();
 	/* write any value to push engine to read a pd */
@@ -1031,6 +1037,11 @@ static void crypto4xx_bh_tasklet_cb(unsigned long data)
 		     ((READ_ONCE(pd->pd_ctl.w) &
 		       (PD_CTL_PE_DONE | PD_CTL_HOST_READY)) ==
 		       PD_CTL_PE_DONE)) {
+			/* order the PE_DONE flag read above before reading
+			 * the descriptor status and output data written by
+			 * the device
+			 */
+			dma_rmb();
 			crypto4xx_pd_done(core_dev->dev, tail);
 			tail = crypto4xx_put_pd_to_pdr(core_dev->dev, tail);
 		} else {
-- 
2.55.0

Re: [PATCHv2] crypto: amcc - fix missing DMA memory barriers in descriptor handling

[Herbert Xu]

On Mon, Aug 10, 2026 at 09:47:38PM -0700, Rosen Penev wrote:
> Fix two memory ordering bugs in the AMCC crypto driver:
> 
> 1. In crypto4xx_bh_tasklet_cb(), add a dma_rmb() between reading the
>    PD_CTL_PE_DONE bit (with READ_ONCE) and reading the descriptor and DMA
>    buffer data in crypto4xx_pd_done().  Without it, the CPU on a
>    weakly-ordered architecture could read stale descriptor data before the
>    hardware's writes are globally visible.
> 
> 2. In crypto4xx_build_pd(), add a dma_wmb() before writing
>    PD_CTL_HOST_READY to ensure all descriptor and SA data is visible to
>    the device before the ownership handover bit.  Also fix the descriptor
>    field ordering: pd_ctl_len.w must be written before pd_ctl.w (which
>    contains HOST_READY), not after, to prevent the hardware from fetching
>    uninitialized length data.
> 
> Fixes: f6c48b76daa6 (crypto: amcc - Add crypto4xx-aead cryptographic
>   AEAD accelerator driver)
> Cc: stable@vger.kernel.org
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  v2: add descriptions for write barriers.
>  drivers/crypto/amcc/crypto4xx_core.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c
> index fd010bfb7020..4b0ce165c198 100644
> --- a/drivers/crypto/amcc/crypto4xx_core.c
> +++ b/drivers/crypto/amcc/crypto4xx_core.c
> @@ -876,11 +876,17 @@ int crypto4xx_build_pd(struct crypto_async_request *req,
>  		}
>  	}
>  
> +	pd->pd_ctl_len.w = 0x00400000 | (assoclen + datalen);
> +	pd_uinfo->state = PD_ENTRY_INUSE | (is_busy ? PD_ENTRY_BUSY : 0);
> +
> +	/* make the pd_ctl_len and pd_uinfo->state writes above visible to
> +	 * the device before the HOST_READY handover write below, so the
> +	 * device never fetches a descriptor with stale length/state bits
> +	 */
> +	dma_wmb();
>  	pd->pd_ctl.w = PD_CTL_HOST_READY |
>  		((crypto_tfm_alg_type(req->tfm) == CRYPTO_ALG_TYPE_AEAD) ?
>  			PD_CTL_HASH_FINAL : 0);
> -	pd->pd_ctl_len.w = 0x00400000 | (assoclen + datalen);
> -	pd_uinfo->state = PD_ENTRY_INUSE | (is_busy ? PD_ENTRY_BUSY : 0);
>  
>  	wmb();
>  	/* write any value to push engine to read a pd */

I would've thought that the hardware is only able to start reading
after this wmb() and the subsequent writes.  If this isn't the case
then plesae explain how the whole sequence works.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt