[v2] crypto: atmel-tdes - sync output bounce buffer before DMA

[PATCH v2] crypto: atmel-tdes - sync output bounce buffer before DMA

Posted by Karl Mehltretter 3 weeks, 2 days ago

The slow path maps its output bounce buffer once at probe time with
dma_map_single() and DMA_FROM_DEVICE, then reuses the mapping for every
request.  After the CPU copies a result from the buffer,
dma_sync_single_for_device() must hand the buffer back to the device before
the next DMA transfer.  The driver omits this call, so cache lines from the
previous result can remain valid while the device writes the next one.

This bug was masked by the completion paths calling
dma_sync_single_for_device() immediately before the CPU copied the output,
where dma_sync_single_for_cpu() was required.  For DMA_FROM_DEVICE on
ARM926, dma_sync_single_for_device() invokes arm926_dma_map_area(), which
invalidates the cache lines.  The misplaced call therefore discarded the
stale lines before every copy-out.

Commit c8a9a647532f ("crypto: atmel-tdes - fix DMA sync direction")
correctly changed the completion paths to call dma_sync_single_for_cpu().
On ARM926, that function invokes arm926_dma_unmap_area(), which is a no-op.
The missing pre-DMA dma_sync_single_for_device() was therefore exposed on
ARM926-based SAM9X60 and SAM9X7 SoCs.

With CONFIG_CRYPTO_SELFTESTS=y all four DES/TDES algorithms fail on
SAM9X75:

  alg: skcipher: atmel-ecb-tdes encryption test failed (wrong result) on
  test vector 2, cfg="unaligned buffer, offset=1"

Call dma_sync_single_for_device() for the output buffer before starting DMA
in both atmel_tdes_crypt_pdc() and atmel_tdes_crypt_dma().

Fixes: c8a9a647532f ("crypto: atmel-tdes - fix DMA sync direction")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Changes in v2:
- Reword the changelog and use full function names. No code changes.
  (Thorsten)

Link to v1:
  https://lore.kernel.org/r/20260829045316.92931-1-kmehltretter@gmail.com/

Tested on top of:

  crypto: atmel-tdes - zero-initialize device state
  https://lore.kernel.org/r/20260829035821.67220-1-kmehltretter@gmail.com/

Without that fix, on the tested SAM9X75 the DES/TDES self-tests hang on
their first requests before reaching this test vector, so the failure
fixed here is not observable on an otherwise unpatched tree.

The two patches are independent and apply in either order.

 drivers/crypto/atmel-tdes.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c
index 2756dab3f4c7..ed80423b4209 100644
--- a/drivers/crypto/atmel-tdes.c
+++ b/drivers/crypto/atmel-tdes.c
@@ -370,6 +370,8 @@ static int atmel_tdes_crypt_pdc(struct atmel_tdes_dev *dd,
 	if (!(dd->flags & TDES_FLAGS_FAST)) {
 		dma_sync_single_for_device(dd->dev, dma_addr_in, length,
 					   DMA_TO_DEVICE);
+		dma_sync_single_for_device(dd->dev, dma_addr_out, length,
+					   DMA_FROM_DEVICE);
 	}
 
 	len32 = DIV_ROUND_UP(length, sizeof(u32));
@@ -402,6 +404,8 @@ static int atmel_tdes_crypt_dma(struct atmel_tdes_dev *dd,
 	if (!(dd->flags & TDES_FLAGS_FAST)) {
 		dma_sync_single_for_device(dd->dev, dma_addr_in, length,
 					   DMA_TO_DEVICE);
+		dma_sync_single_for_device(dd->dev, dma_addr_out, length,
+					   DMA_FROM_DEVICE);
 	}
 
 	addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v2] crypto: atmel-tdes - sync output bounce buffer before DMA

Posted by Herbert Xu 1 week, 2 days ago

On Wed, Sep 02, 2026 at 09:23:23PM +0200, Karl Mehltretter wrote:
> The slow path maps its output bounce buffer once at probe time with
> dma_map_single() and DMA_FROM_DEVICE, then reuses the mapping for every
> request.  After the CPU copies a result from the buffer,
> dma_sync_single_for_device() must hand the buffer back to the device before
> the next DMA transfer.  The driver omits this call, so cache lines from the
> previous result can remain valid while the device writes the next one.
> 
> This bug was masked by the completion paths calling
> dma_sync_single_for_device() immediately before the CPU copied the output,
> where dma_sync_single_for_cpu() was required.  For DMA_FROM_DEVICE on
> ARM926, dma_sync_single_for_device() invokes arm926_dma_map_area(), which
> invalidates the cache lines.  The misplaced call therefore discarded the
> stale lines before every copy-out.
> 
> Commit c8a9a647532f ("crypto: atmel-tdes - fix DMA sync direction")
> correctly changed the completion paths to call dma_sync_single_for_cpu().
> On ARM926, that function invokes arm926_dma_unmap_area(), which is a no-op.
> The missing pre-DMA dma_sync_single_for_device() was therefore exposed on
> ARM926-based SAM9X60 and SAM9X7 SoCs.
> 
> With CONFIG_CRYPTO_SELFTESTS=y all four DES/TDES algorithms fail on
> SAM9X75:
> 
>   alg: skcipher: atmel-ecb-tdes encryption test failed (wrong result) on
>   test vector 2, cfg="unaligned buffer, offset=1"
> 
> Call dma_sync_single_for_device() for the output buffer before starting DMA
> in both atmel_tdes_crypt_pdc() and atmel_tdes_crypt_dma().
> 
> Fixes: c8a9a647532f ("crypto: atmel-tdes - fix DMA sync direction")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> Changes in v2:
> - Reword the changelog and use full function names. No code changes.
>   (Thorsten)
> 
> Link to v1:
>   https://lore.kernel.org/r/20260829045316.92931-1-kmehltretter@gmail.com/
> 
> Tested on top of:
> 
>   crypto: atmel-tdes - zero-initialize device state
>   https://lore.kernel.org/r/20260829035821.67220-1-kmehltretter@gmail.com/
> 
> Without that fix, on the tested SAM9X75 the DES/TDES self-tests hang on
> their first requests before reaching this test vector, so the failure
> fixed here is not observable on an otherwise unpatched tree.
> 
> The two patches are independent and apply in either order.
> 
>  drivers/crypto/atmel-tdes.c | 4 ++++
>  1 file changed, 4 insertions(+)

Patch applied.  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

Re: [PATCH v2] crypto: atmel-tdes - sync output bounce buffer before DMA

Posted by Thorsten Blum 3 weeks, 1 day ago

On Wed, Sep 02, 2026 at 09:23:23PM +0200, Karl Mehltretter wrote:
> The slow path maps its output bounce buffer once at probe time with
> dma_map_single() and DMA_FROM_DEVICE, then reuses the mapping for every
> request.  After the CPU copies a result from the buffer,
> dma_sync_single_for_device() must hand the buffer back to the device before
> the next DMA transfer.  The driver omits this call, so cache lines from the
> previous result can remain valid while the device writes the next one.
> 
> This bug was masked by the completion paths calling
> dma_sync_single_for_device() immediately before the CPU copied the output,
> where dma_sync_single_for_cpu() was required.  For DMA_FROM_DEVICE on
> ARM926, dma_sync_single_for_device() invokes arm926_dma_map_area(), which
> invalidates the cache lines.  The misplaced call therefore discarded the
> stale lines before every copy-out.
> 
> Commit c8a9a647532f ("crypto: atmel-tdes - fix DMA sync direction")
> correctly changed the completion paths to call dma_sync_single_for_cpu().
> On ARM926, that function invokes arm926_dma_unmap_area(), which is a no-op.
> The missing pre-DMA dma_sync_single_for_device() was therefore exposed on
> ARM926-based SAM9X60 and SAM9X7 SoCs.
> 
> With CONFIG_CRYPTO_SELFTESTS=y all four DES/TDES algorithms fail on
> SAM9X75:
> 
>   alg: skcipher: atmel-ecb-tdes encryption test failed (wrong result) on
>   test vector 2, cfg="unaligned buffer, offset=1"
> 
> Call dma_sync_single_for_device() for the output buffer before starting DMA
> in both atmel_tdes_crypt_pdc() and atmel_tdes_crypt_dma().
> 
> Fixes: c8a9a647532f ("crypto: atmel-tdes - fix DMA sync direction")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> Changes in v2:
> - Reword the changelog and use full function names. No code changes.
>   (Thorsten)
> 
> Link to v1:
>   https://lore.kernel.org/r/20260829045316.92931-1-kmehltretter@gmail.com/
> 
> Tested on top of:
> 
>   crypto: atmel-tdes - zero-initialize device state
>   https://lore.kernel.org/r/20260829035821.67220-1-kmehltretter@gmail.com/
> 
> Without that fix, on the tested SAM9X75 the DES/TDES self-tests hang on
> their first requests before reaching this test vector, so the failure
> fixed here is not observable on an otherwise unpatched tree.
> 
> The two patches are independent and apply in either order.
> 
>  drivers/crypto/atmel-tdes.c | 4 ++++
>  1 file changed, 4 insertions(+)

LGTM, thanks.

Reviewed-by: Thorsten Blum <blum@kernel.org>