Re: [PATCH v2 2/5] crypto: iaa - fall back to software for multi-entry scatterlists

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Software decompression fallback reads and writes buffers while they are still DMA-mapped, leading to data corruption on systems using SWIOTLB (bounce buffers).
--

--- Patch [2]: [PATCH v2 2/5] crypto: iaa - fall back to software for multi-entry scatterlists ---
commit 6596711a82477095464512f0a5cfb3f894ac8fad
Author: Giovanni Cabiddu <[email protected]>
Subject: crypto: iaa - fall back to software for multi-entry scatterlists

This commit routes multi-entry scatterlists to a software deflate
implementation instead of failing them. It simplifies the DMA mapping
paths to handle only single entries and falls back to software on mapping
failures. Counters are added to track the number of software fallback
compressions.

Cc: [email protected]

> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> index f62b994e18e58..904d9413ba182 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c

[ ... ]

> @@ -1639,10 +1650,8 @@ static int iaa_comp_adecompress(struct acomp_req *req)
>  	if (ret != 0)
>  		dev_dbg(dev, "asynchronous decompress failed ret=%d\n", ret);
>  
> -	dma_unmap_sg(dev, req->dst, sg_nents(req->dst), DMA_FROM_DEVICE);
> -err_map_dst:
> -	dma_unmap_sg(dev, req->src, sg_nents(req->src), DMA_TO_DEVICE);
> -out:
> +	dma_unmap_sg(dev, req->dst, 1, DMA_FROM_DEVICE);
> +	dma_unmap_sg(dev, req->src, 1, DMA_TO_DEVICE);
>  	iaa_wq_put(wq);
>  
>  	return ret;

[Severity: High]
This is a pre-existing issue, but can the software fallback logic corrupt
data on systems using SWIOTLB (bounce buffers)?

If hardware decompression fails with an analytics error (for example,
IAA_ANALYTICS_ERROR), the driver falls back to software decompression via
deflate_generic_decompress(). This fallback executes directly on the CPU,
modifying the original virtual pages.

In the async completion path, the software fallback is executed while the
buffers are still actively DMA-mapped:

iaa_crypto_main.c:iaa_desc_complete() {
        ...
        if (err == IAA_ANALYTICS_ERROR) {
                ret = deflate_generic_decompress(ctx->req);
        ...
        dma_unmap_sg(dev, req->dst, 1, DMA_FROM_DEVICE);
}

When SWIOTLB is active, this unmap operation syncs the bounce buffer back
to the original memory. Does this completely overwrite the correct data just
produced by the software fallback with stale hardware garbage?

Could the driver unmap the buffers before falling back to software
decompression to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2