Re: [PATCH v2 4/5] crypto: iaa - use bounce buffer for multi-sg decompress input

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

Pre-existing issues:
- [Critical] Software fallback inside the synchronous `iaa_decompress()` path writes to `req->dst` while it remains mapped for DMA, causing data corruption when unmapped if SWIOTLB is active.
--

--- Patch [4]: [PATCH v2 4/5] crypto: iaa - use bounce buffer for multi-sg decompress input ---
commit 734c3b6227baae1deae54e3c6e85848a75b44265
Author: Giovanni Cabiddu <[email protected]>

crypto: iaa - use bounce buffer for multi-sg decompress input

This commit introduces a pre-allocated bounce page to linearize small 
multi-entry sources for IAA hardware decompression. This avoids falling 
back to software for multi-entry sources while maintaining the fallback 
for multi-entry destinations.

> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> index 9505ca23e6f38..51547c5fcf701 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c

[ ... ]

> @@ -1653,7 +1726,7 @@ static int iaa_comp_adecompress(struct acomp_req *req)
>  		dev_dbg(dev, "asynchronous decompress failed ret=%d\n", ret);
>  
>  	dma_unmap_sg(dev, req->dst, 1, DMA_FROM_DEVICE);
> -	dma_unmap_sg(dev, req->src, 1, DMA_TO_DEVICE);
> +	iaa_unmap_src(dev, req);
>  	iaa_wq_put(wq);

[Severity: Critical]
This is a pre-existing issue, but since this patch modifies the unmap
path here, I noticed a potential data corruption issue when SWIOTLB is
active.

If iaa_decompress() fails and falls back to software via
deflate_generic_decompress(req), the CPU will write valid decompressed
data directly to req->dst.

However, req->dst is still mapped for DMA (DMA_FROM_DEVICE). When
iaa_decompress() returns success after the fallback, we reach this
dma_unmap_sg() in iaa_comp_adecompress():

iaa_comp_adecompress()
   dma_map_sg(..., req->dst, DMA_FROM_DEVICE);
   ...
   ret = iaa_decompress(...)
      -> hardware fails, falls back to deflate_generic_decompress(req)
      -> CPU writes decompressed data to req->dst
      -> returns 0

   dma_unmap_sg(..., req->dst, DMA_FROM_DEVICE);

If SWIOTLB is active, won't this dma_unmap_sg() copy the unmodified or
partially modified bounce buffer back over req->dst, overwriting the
valid CPU-written data with garbage?

Should the software fallback inside iaa_decompress() unmap req->dst before
calling deflate_generic_decompress(req), similar to how
iaa_desc_complete() handles its fallback?

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