[PATCH 0/4] crypto: iaa - Fixes for multi entry SG lists

[PATCH 0/4] crypto: iaa - Fixes for multi entry SG lists

[Vinicius Costa Gomes]

Since commit e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs
from zsmalloc"), iaa_crypto started seeing some failures with
multi-entry scatter lists.

For that we introduce software fallback, in patch 2/4, to iaa-crypto
when SG lists have more than one entry, for both input and output.
Patch 4/4 adds a bounce buffer so small/simple requests can be
linearized and sent to the hardware. This recovers most of the
performance.

Patch 1/4 updates the default resources reserved to iaa-crypto so more
engines are associated to the iaa_crypto group, resulting in better
utilization by default. Patch 3/4 fixes so software requests are not
double counted. As the idxd changes only affect iaa_crypto, sending
them here makes more sense.

It should be noted that as the software and hardware implementations
have different expectations for the window size, something like patch
[1] or the future 'set_params()' API are needed to verify that patch
2/4 works without patch 4/4.

[1] https://lore.kernel.org/linux-crypto/20260326100433.57324-1-giovanni.cabiddu@intel.com/

Cheers,

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
Giovanni Cabiddu (4):
      dmaengine: idxd: assign all engines to group 0 in IAA defaults
      crypto: iaa - fall back to software for multi-entry scatterlists
      crypto: iaa - avoid counting fallback decompression bytes
      crypto: iaa - use bounce buffer for multi-sg decompress input

 drivers/crypto/intel/iaa/iaa_crypto_main.c  | 236 ++++++++++++++++++++--------
 drivers/crypto/intel/iaa/iaa_crypto_stats.c |  11 +-
 drivers/crypto/intel/iaa/iaa_crypto_stats.h |   2 +
 drivers/dma/idxd/defaults.c                 |  12 +-
 4 files changed, 186 insertions(+), 75 deletions(-)
---
base-commit: b73b71df4cb4ca241165ad31218c82dfe489147c
change-id: 20260713-iaa-crypto-fixes-zswap-ff5baae311d1

Best regards,
--  
Vinicius

[PATCH 1/4] dmaengine: idxd: assign all engines to group 0 in IAA defaults

[Vinicius Costa Gomes]

From: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

The IAA device defaults only assigned engine 0 to group 0, leaving
engines 1 through max_engines-1 unassigned (group_id = -1). This means
that by default only a single engine processed descriptors, limiting
throughput to one engine's capacity.

Assign all available engines to group 0 so that the full hardware
parallelism is used out of the box without requiring manual
accel-config setup.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/dma/idxd/defaults.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/idxd/defaults.c b/drivers/dma/idxd/defaults.c
index 2bbbcd02a0da..26ebfa2ca144 100644
--- a/drivers/dma/idxd/defaults.c
+++ b/drivers/dma/idxd/defaults.c
@@ -8,6 +8,7 @@ int idxd_load_iaa_device_defaults(struct idxd_device *idxd)
 	struct idxd_engine *engine;
 	struct idxd_group *group;
 	struct idxd_wq *wq;
+	int i;
 
 	if (!test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags))
 		return 0;
@@ -41,11 +42,12 @@ int idxd_load_iaa_device_defaults(struct idxd_device *idxd)
 	/* set driver_name to "crypto" */
 	strscpy_pad(wq->driver_name, "crypto");
 
-	engine = idxd->engines[0];
-
-	/* set engine group to 0 */
-	engine->group = idxd->groups[0];
-	engine->group->num_engines++;
+	/* assign all engines to group 0 */
+	for (i = 0; i < idxd->max_engines; i++) {
+		engine = idxd->engines[i];
+		engine->group = group;
+		group->num_engines++;
+	}
 
 	return 0;
 }

-- 
2.55.0

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

[Vinicius Costa Gomes]

From: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

IAA cannot process source or destination scatterlists with more than one
entry directly. Instead of failing these requests, route them through a
separate deflate acomp transform and keep the request alive in software.

Since commit e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs
from zsmalloc"), zswap passes the raw zsmalloc SG list directly to
crypto drivers, so objects spanning multiple pages now reach IAA as
multi-entry sources and would otherwise fail decompression.

Fallback to the generic DEFLATE implementation for scatterlists with
more than one entry. After the multi-entry cases fall back early,
simplify the DMA mapping path to a single scatterlist entry and fall
back on mapping failure as well.

Add counters to track the number of bytes processed by the software
implementation on the compression direction.

Fixes: e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs from zsmalloc")
Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/intel/iaa/iaa_crypto_main.c  | 122 ++++++++++++++++------------
 drivers/crypto/intel/iaa/iaa_crypto_stats.c |  11 ++-
 drivers/crypto/intel/iaa/iaa_crypto_stats.h |   2 +
 3 files changed, 84 insertions(+), 51 deletions(-)

diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
index f62b994e18e5..fb154959c2aa 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
@@ -2,6 +2,7 @@
 /* Copyright(c) 2021 Intel Corporation. All rights rsvd. */
 
 #include <linux/init.h>
+#include <linux/crypto.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -983,17 +984,43 @@ static inline int check_completion(struct device *dev,
 	return ret;
 }
 
-static int deflate_generic_decompress(struct acomp_req *req)
+static int deflate_fallback(struct acomp_req *req, bool compress)
 {
 	ACOMP_FBREQ_ON_STACK(fbreq, req);
 	int ret;
 
-	ret = crypto_acomp_decompress(fbreq);
+	ret = compress ?
+		crypto_acomp_compress(fbreq) :
+		crypto_acomp_decompress(fbreq);
 	req->dlen = fbreq->dlen;
 
+	return ret;
+}
+
+static int deflate_generic_decompress(struct acomp_req *req)
+{
+	int ret;
+
+	ret = deflate_fallback(req, false);
+	if (ret)
+		return ret;
+
 	update_total_sw_decomp_calls();
 
-	return ret;
+	return 0;
+}
+
+static int deflate_generic_compress(struct acomp_req *req)
+{
+	int ret;
+
+	ret = deflate_fallback(req, true);
+	if (ret)
+		return ret;
+
+	update_total_sw_comp_calls();
+
+	return 0;
 }
 
 static int iaa_remap_for_verify(struct device *dev, struct iaa_wq *iaa_wq,
@@ -1472,7 +1499,7 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	struct iaa_compression_ctx *compression_ctx;
 	struct crypto_tfm *tfm = req->base.tfm;
 	dma_addr_t src_addr, dst_addr;
-	int nr_sgs, cpu, ret = 0;
+	int cpu, ret = 0;
 	struct iaa_wq *iaa_wq;
 	struct idxd_wq *wq;
 	struct device *dev;
@@ -1489,6 +1516,10 @@ static int iaa_comp_acompress(struct acomp_req *req)
 		return -EINVAL;
 	}
 
+	/* Fall back to software if src or dst has multiple sg entries */
+	if (sg_nents(req->src) > 1 || sg_nents(req->dst) > 1)
+		return deflate_generic_compress(req);
+
 	cpu = get_cpu();
 	wq = wq_table_next_wq(cpu);
 	put_cpu();
@@ -1507,30 +1538,25 @@ static int iaa_comp_acompress(struct acomp_req *req)
 
 	dev = &wq->idxd->pdev->dev;
 
-	nr_sgs = dma_map_sg(dev, req->src, sg_nents(req->src), DMA_TO_DEVICE);
-	if (nr_sgs <= 0 || nr_sgs > 1) {
-		dev_dbg(dev, "couldn't map src sg for iaa device %d,"
-			" wq %d: ret=%d\n", iaa_wq->iaa_device->idxd->id,
-			iaa_wq->wq->id, ret);
-		ret = -EIO;
-		goto out;
+	if (!dma_map_sg(dev, req->src, 1, DMA_TO_DEVICE)) {
+		dev_dbg(dev, "couldn't map src sg for iaa device %d, wq %d\n",
+			iaa_wq->iaa_device->idxd->id, iaa_wq->wq->id);
+		iaa_wq_put(wq);
+		return deflate_generic_compress(req);
 	}
 	src_addr = sg_dma_address(req->src);
-	dev_dbg(dev, "dma_map_sg, src_addr %llx, nr_sgs %d, req->src %p,"
-		" req->slen %d, sg_dma_len(sg) %d\n", src_addr, nr_sgs,
+	dev_dbg(dev, "map src %llx req->src %p slen %d sg_len %d\n", src_addr,
 		req->src, req->slen, sg_dma_len(req->src));
 
-	nr_sgs = dma_map_sg(dev, req->dst, sg_nents(req->dst), DMA_FROM_DEVICE);
-	if (nr_sgs <= 0 || nr_sgs > 1) {
-		dev_dbg(dev, "couldn't map dst sg for iaa device %d,"
-			" wq %d: ret=%d\n", iaa_wq->iaa_device->idxd->id,
-			iaa_wq->wq->id, ret);
-		ret = -EIO;
-		goto err_map_dst;
+	if (!dma_map_sg(dev, req->dst, 1, DMA_FROM_DEVICE)) {
+		dev_dbg(dev, "couldn't map dst sg for iaa device %d, wq %d\n",
+			iaa_wq->iaa_device->idxd->id, iaa_wq->wq->id);
+		dma_unmap_sg(dev, req->src, 1, DMA_TO_DEVICE);
+		iaa_wq_put(wq);
+		return deflate_generic_compress(req);
 	}
 	dst_addr = sg_dma_address(req->dst);
-	dev_dbg(dev, "dma_map_sg, dst_addr %llx, nr_sgs %d, req->dst %p,"
-		" req->dlen %d, sg_dma_len(sg) %d\n", dst_addr, nr_sgs,
+	dev_dbg(dev, "map dst %llx req->dst %p dlen %d sg_len %d\n", dst_addr,
 		req->dst, req->dlen, sg_dma_len(req->dst));
 
 	ret = iaa_compress(tfm, req, wq, src_addr, req->slen, dst_addr,
@@ -1550,8 +1576,8 @@ static int iaa_comp_acompress(struct acomp_req *req)
 		if (ret)
 			dev_dbg(dev, "asynchronous compress verification failed ret=%d\n", ret);
 
-		dma_unmap_sg(dev, req->dst, sg_nents(req->dst), DMA_TO_DEVICE);
-		dma_unmap_sg(dev, req->src, sg_nents(req->src), DMA_FROM_DEVICE);
+		dma_unmap_sg(dev, req->dst, 1, DMA_TO_DEVICE);
+		dma_unmap_sg(dev, req->src, 1, DMA_FROM_DEVICE);
 
 		goto out;
 	}
@@ -1559,9 +1585,8 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	if (ret)
 		dev_dbg(dev, "asynchronous compress 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);
+	dma_unmap_sg(dev, req->dst, 1, DMA_FROM_DEVICE);
+	dma_unmap_sg(dev, req->src, 1, DMA_TO_DEVICE);
 out:
 	iaa_wq_put(wq);
 
@@ -1572,7 +1597,7 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 {
 	struct crypto_tfm *tfm = req->base.tfm;
 	dma_addr_t src_addr, dst_addr;
-	int nr_sgs, cpu, ret = 0;
+	int cpu, ret = 0;
 	struct iaa_wq *iaa_wq;
 	struct device *dev;
 	struct idxd_wq *wq;
@@ -1587,6 +1612,10 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 		return -EINVAL;
 	}
 
+	/* Fall back to software if src or dst has multiple sg entries */
+	if (sg_nents(req->src) > 1 || sg_nents(req->dst) > 1)
+		return deflate_generic_decompress(req);
+
 	cpu = get_cpu();
 	wq = wq_table_next_wq(cpu);
 	put_cpu();
@@ -1605,30 +1634,25 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 
 	dev = &wq->idxd->pdev->dev;
 
-	nr_sgs = dma_map_sg(dev, req->src, sg_nents(req->src), DMA_TO_DEVICE);
-	if (nr_sgs <= 0 || nr_sgs > 1) {
-		dev_dbg(dev, "couldn't map src sg for iaa device %d,"
-			" wq %d: ret=%d\n", iaa_wq->iaa_device->idxd->id,
-			iaa_wq->wq->id, ret);
-		ret = -EIO;
-		goto out;
+	if (!dma_map_sg(dev, req->src, 1, DMA_TO_DEVICE)) {
+		dev_dbg(dev, "couldn't map src sg for iaa device %d, wq %d\n",
+			iaa_wq->iaa_device->idxd->id, iaa_wq->wq->id);
+		iaa_wq_put(wq);
+		return deflate_generic_decompress(req);
 	}
 	src_addr = sg_dma_address(req->src);
-	dev_dbg(dev, "dma_map_sg, src_addr %llx, nr_sgs %d, req->src %p,"
-		" req->slen %d, sg_dma_len(sg) %d\n", src_addr, nr_sgs,
+	dev_dbg(dev, "map src %llx req->src %p slen %d sg_len %d\n", src_addr,
 		req->src, req->slen, sg_dma_len(req->src));
 
-	nr_sgs = dma_map_sg(dev, req->dst, sg_nents(req->dst), DMA_FROM_DEVICE);
-	if (nr_sgs <= 0 || nr_sgs > 1) {
-		dev_dbg(dev, "couldn't map dst sg for iaa device %d,"
-			" wq %d: ret=%d\n", iaa_wq->iaa_device->idxd->id,
-			iaa_wq->wq->id, ret);
-		ret = -EIO;
-		goto err_map_dst;
+	if (!dma_map_sg(dev, req->dst, 1, DMA_FROM_DEVICE)) {
+		dev_dbg(dev, "couldn't map dst sg for iaa device %d, wq %d\n",
+			iaa_wq->iaa_device->idxd->id, iaa_wq->wq->id);
+		dma_unmap_sg(dev, req->src, 1, DMA_TO_DEVICE);
+		iaa_wq_put(wq);
+		return deflate_generic_decompress(req);
 	}
 	dst_addr = sg_dma_address(req->dst);
-	dev_dbg(dev, "dma_map_sg, dst_addr %llx, nr_sgs %d, req->dst %p,"
-		" req->dlen %d, sg_dma_len(sg) %d\n", dst_addr, nr_sgs,
+	dev_dbg(dev, "map dst %llx req->dst %p dlen %d sg_len %d\n", dst_addr,
 		req->dst, req->dlen, sg_dma_len(req->dst));
 
 	ret = iaa_decompress(tfm, req, wq, src_addr, req->slen,
@@ -1639,10 +1663,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;
diff --git a/drivers/crypto/intel/iaa/iaa_crypto_stats.c b/drivers/crypto/intel/iaa/iaa_crypto_stats.c
index f5cc3d29ca19..55d0d9c6f61a 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_stats.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_stats.c
@@ -19,6 +19,7 @@
 
 static atomic64_t total_comp_calls;
 static atomic64_t total_decomp_calls;
+static atomic64_t total_sw_comp_calls;
 static atomic64_t total_sw_decomp_calls;
 static atomic64_t total_comp_bytes_out;
 static atomic64_t total_decomp_bytes_in;
@@ -43,6 +44,11 @@ void update_total_decomp_calls(void)
 	atomic64_inc(&total_decomp_calls);
 }
 
+void update_total_sw_comp_calls(void)
+{
+	atomic64_inc(&total_sw_comp_calls);
+}
+
 void update_total_sw_decomp_calls(void)
 {
 	atomic64_inc(&total_sw_decomp_calls);
@@ -104,6 +110,7 @@ static void reset_iaa_crypto_stats(void)
 {
 	atomic64_set(&total_comp_calls, 0);
 	atomic64_set(&total_decomp_calls, 0);
+	atomic64_set(&total_sw_comp_calls, 0);
 	atomic64_set(&total_sw_decomp_calls, 0);
 	atomic64_set(&total_comp_bytes_out, 0);
 	atomic64_set(&total_decomp_bytes_in, 0);
@@ -174,6 +181,8 @@ static int global_stats_show(struct seq_file *m, void *v)
 		   atomic64_read(&total_comp_calls));
 	seq_printf(m, "  total_decomp_calls: %llu\n",
 		   atomic64_read(&total_decomp_calls));
+	seq_printf(m, "  total_sw_comp_calls: %llu\n",
+		   atomic64_read(&total_sw_comp_calls));
 	seq_printf(m, "  total_sw_decomp_calls: %llu\n",
 		   atomic64_read(&total_sw_decomp_calls));
 	seq_printf(m, "  total_comp_bytes_out: %llu\n",
@@ -263,7 +272,7 @@ int __init iaa_crypto_debugfs_init(void)
 	return 0;
 }
 
-void __exit iaa_crypto_debugfs_cleanup(void)
+void iaa_crypto_debugfs_cleanup(void)
 {
 	debugfs_remove_recursive(iaa_crypto_debugfs_root);
 }
diff --git a/drivers/crypto/intel/iaa/iaa_crypto_stats.h b/drivers/crypto/intel/iaa/iaa_crypto_stats.h
index 3787a5f507eb..6e0c6f9939bf 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_stats.h
+++ b/drivers/crypto/intel/iaa/iaa_crypto_stats.h
@@ -11,6 +11,7 @@ void	iaa_crypto_debugfs_cleanup(void);
 void	update_total_comp_calls(void);
 void	update_total_comp_bytes_out(int n);
 void	update_total_decomp_calls(void);
+void	update_total_sw_comp_calls(void);
 void	update_total_sw_decomp_calls(void);
 void	update_total_decomp_bytes_in(int n);
 void	update_completion_einval_errs(void);
@@ -29,6 +30,7 @@ static inline void	iaa_crypto_debugfs_cleanup(void) {}
 static inline void	update_total_comp_calls(void) {}
 static inline void	update_total_comp_bytes_out(int n) {}
 static inline void	update_total_decomp_calls(void) {}
+static inline void	update_total_sw_comp_calls(void) {}
 static inline void	update_total_sw_decomp_calls(void) {}
 static inline void	update_total_decomp_bytes_in(int n) {}
 static inline void	update_completion_einval_errs(void) {}

-- 
2.55.0

[PATCH 3/4] crypto: iaa - avoid counting fallback decompression bytes

[Vinicius Costa Gomes]

From: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

When decompression falls back to deflate-generic after an analytics
error, the request no longer completes through IAA.

Move decompression byte accounting into the successful IAA completion
path in both the synchronous and asynchronous flows so decomp_bytes only
reflects bytes actually processed by IAA.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/intel/iaa/iaa_crypto_main.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
index fb154959c2aa..8f68b1478476 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
@@ -1084,15 +1084,17 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
 		}
 	} else {
 		ctx->req->dlen = idxd_desc->iax_completion->output_size;
+
+		if (!ctx->compress) {
+			update_total_decomp_bytes_in(ctx->req->slen);
+			update_wq_decomp_bytes(iaa_wq->wq, ctx->req->slen);
+		}
 	}
 
 	/* Update stats */
 	if (ctx->compress) {
 		update_total_comp_bytes_out(ctx->req->dlen);
 		update_wq_comp_bytes(iaa_wq->wq, ctx->req->dlen);
-	} else {
-		update_total_decomp_bytes_in(ctx->req->slen);
-		update_wq_decomp_bytes(iaa_wq->wq, ctx->req->slen);
 	}
 
 	if (ctx->compress && compression_ctx->verify_compress) {
@@ -1475,16 +1477,16 @@ static int iaa_decompress(struct crypto_tfm *tfm, struct acomp_req *req,
 		}
 	} else {
 		req->dlen = idxd_desc->iax_completion->output_size;
+
+		/* Update stats */
+		update_total_decomp_bytes_in(slen);
+		update_wq_decomp_bytes(wq, slen);
 	}
 
 	*dlen = req->dlen;
 
 	if (!ctx->async_mode)
 		idxd_free_desc(wq, idxd_desc);
-
-	/* Update stats */
-	update_total_decomp_bytes_in(slen);
-	update_wq_decomp_bytes(wq, slen);
 out:
 	return ret;
 err:

-- 
2.55.0

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

[Vinicius Costa Gomes]

From: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

Since commit e2c3b6b21c77 ("mm: zswap: use SG list decompression APIs
from zsmalloc"), zswap passes the raw zsmalloc SG list directly to
crypto drivers, so a compressed object spanning multiple pages reaches
IAA as a multi-entry source. Such requests currently fall back to
software decompression.

As IAA hardware requires a single DMA source buffer, linearize small
multi-entry sources into a pre-allocated bounce page and submit that to
the hardware instead of falling back to software. Keep the software
fallback only for multi-entry destinations. This recovers most of the
performance lost by using the software fallback.

Store the bounce-page state in the acomp request context alongside the
existing compression CRC, free it through a shared source-unmap helper,
and back the pages with a small module-wide mempool so the path remains
available in reclaim-driven callers.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/intel/iaa/iaa_crypto_main.c | 110 ++++++++++++++++++++++++-----
 1 file changed, 92 insertions(+), 18 deletions(-)

diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
index 8f68b1478476..54bde11c454c 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
@@ -9,6 +9,7 @@
 #include <linux/sysfs.h>
 #include <linux/device.h>
 #include <linux/iommu.h>
+#include <linux/mempool.h>
 #include <uapi/linux/idxd.h>
 #include <linux/highmem.h>
 #include <linux/sched/smt.h>
@@ -157,6 +158,15 @@ static bool async_mode;
 /* Use interrupts */
 static bool use_irq;
 
+struct iaa_req_ctx {
+	u32 compression_crc;
+	struct page *bounce_src;
+	struct scatterlist bounce_sg;
+};
+
+static mempool_t *iaa_bounce_pool;
+#define IAA_BOUNCE_POOL_SIZE	128
+
 /**
  * set_iaa_sync_mode - Set IAA sync mode
  * @name: The name of the sync mode
@@ -984,6 +994,19 @@ static inline int check_completion(struct device *dev,
 	return ret;
 }
 
+static void iaa_unmap_src(struct device *dev, struct acomp_req *req)
+{
+	struct iaa_req_ctx *req_ctx = acomp_request_ctx(req);
+	struct scatterlist *src = req_ctx->bounce_src ? &req_ctx->bounce_sg : req->src;
+
+	dma_unmap_sg(dev, src, 1, DMA_TO_DEVICE);
+
+	if (req_ctx->bounce_src) {
+		mempool_free(req_ctx->bounce_src, iaa_bounce_pool);
+		req_ctx->bounce_src = NULL;
+	}
+}
+
 static int deflate_fallback(struct acomp_req *req, bool compress)
 {
 	ACOMP_FBREQ_ON_STACK(fbreq, req);
@@ -1040,6 +1063,7 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
 	struct iaa_device_compression_mode *active_compression_mode;
 	struct iaa_compression_ctx *compression_ctx;
 	struct crypto_ctx *ctx = __ctx;
+	struct iaa_req_ctx *req_ctx = acomp_request_ctx(ctx->req);
 	struct iaa_device *iaa_device;
 	struct idxd_device *idxd;
 	struct iaa_wq *iaa_wq;
@@ -1098,10 +1122,9 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
 	}
 
 	if (ctx->compress && compression_ctx->verify_compress) {
-		u32 *compression_crc = acomp_request_ctx(ctx->req);
 		dma_addr_t src_addr, dst_addr;
 
-		*compression_crc = idxd_desc->iax_completion->crc;
+		req_ctx->compression_crc = idxd_desc->iax_completion->crc;
 
 		ret = iaa_remap_for_verify(dev, iaa_wq, ctx->req, &src_addr, &dst_addr);
 		if (ret) {
@@ -1124,7 +1147,7 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
 	}
 err:
 	dma_unmap_sg(dev, ctx->req->dst, sg_nents(ctx->req->dst), DMA_FROM_DEVICE);
-	dma_unmap_sg(dev, ctx->req->src, sg_nents(ctx->req->src), DMA_TO_DEVICE);
+	iaa_unmap_src(dev, ctx->req);
 out:
 	if (ret != 0)
 		dev_dbg(dev, "asynchronous compress failed ret=%d\n", ret);
@@ -1144,7 +1167,7 @@ static int iaa_compress(struct crypto_tfm *tfm,	struct acomp_req *req,
 {
 	struct iaa_device_compression_mode *active_compression_mode;
 	struct iaa_compression_ctx *ctx = crypto_tfm_ctx(tfm);
-	u32 *compression_crc = acomp_request_ctx(req);
+	struct iaa_req_ctx *req_ctx = acomp_request_ctx(req);
 	struct iaa_device *iaa_device;
 	struct idxd_desc *idxd_desc;
 	struct iax_hw_desc *desc;
@@ -1235,7 +1258,7 @@ static int iaa_compress(struct crypto_tfm *tfm,	struct acomp_req *req,
 	update_total_comp_bytes_out(*dlen);
 	update_wq_comp_bytes(wq, *dlen);
 
-	*compression_crc = idxd_desc->iax_completion->crc;
+	req_ctx->compression_crc = idxd_desc->iax_completion->crc;
 
 	if (!ctx->async_mode)
 		idxd_free_desc(wq, idxd_desc);
@@ -1295,7 +1318,7 @@ static int iaa_compress_verify(struct crypto_tfm *tfm, struct acomp_req *req,
 {
 	struct iaa_device_compression_mode *active_compression_mode;
 	struct iaa_compression_ctx *ctx = crypto_tfm_ctx(tfm);
-	u32 *compression_crc = acomp_request_ctx(req);
+	struct iaa_req_ctx *req_ctx = acomp_request_ctx(req);
 	struct iaa_device *iaa_device;
 	struct idxd_desc *idxd_desc;
 	struct iax_hw_desc *desc;
@@ -1355,10 +1378,10 @@ static int iaa_compress_verify(struct crypto_tfm *tfm, struct acomp_req *req,
 		goto err;
 	}
 
-	if (*compression_crc != idxd_desc->iax_completion->crc) {
+	if (req_ctx->compression_crc != idxd_desc->iax_completion->crc) {
 		ret = -EINVAL;
-		dev_dbg(dev, "(verify) iaa comp/decomp crc mismatch:"
-			" comp=0x%x, decomp=0x%x\n", *compression_crc,
+		dev_dbg(dev, "(verify) iaa comp/decomp crc mismatch: comp=0x%x, decomp=0x%x\n",
+			req_ctx->compression_crc,
 			idxd_desc->iax_completion->crc);
 		print_hex_dump(KERN_INFO, "cmp-rec: ", DUMP_PREFIX_OFFSET,
 			       8, 1, idxd_desc->iax_completion, 64, 0);
@@ -1498,6 +1521,7 @@ static int iaa_decompress(struct crypto_tfm *tfm, struct acomp_req *req,
 
 static int iaa_comp_acompress(struct acomp_req *req)
 {
+	struct iaa_req_ctx *req_ctx = acomp_request_ctx(req);
 	struct iaa_compression_ctx *compression_ctx;
 	struct crypto_tfm *tfm = req->base.tfm;
 	dma_addr_t src_addr, dst_addr;
@@ -1506,6 +1530,8 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	struct idxd_wq *wq;
 	struct device *dev;
 
+	req_ctx->bounce_src = NULL;
+
 	compression_ctx = crypto_tfm_ctx(tfm);
 
 	if (!iaa_crypto_enabled) {
@@ -1597,12 +1623,18 @@ static int iaa_comp_acompress(struct acomp_req *req)
 
 static int iaa_comp_adecompress(struct acomp_req *req)
 {
+	struct iaa_req_ctx *req_ctx = acomp_request_ctx(req);
 	struct crypto_tfm *tfm = req->base.tfm;
+	struct scatterlist *src = req->src;
 	dma_addr_t src_addr, dst_addr;
+	bool use_bounce_src = false;
 	int cpu, ret = 0;
 	struct iaa_wq *iaa_wq;
 	struct device *dev;
 	struct idxd_wq *wq;
+	struct page *page;
+
+	req_ctx->bounce_src = NULL;
 
 	if (!iaa_crypto_enabled) {
 		pr_debug("iaa_crypto disabled, not decompressing\n");
@@ -1614,10 +1646,16 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 		return -EINVAL;
 	}
 
-	/* Fall back to software if src or dst has multiple sg entries */
-	if (sg_nents(req->src) > 1 || sg_nents(req->dst) > 1)
+	/* Fall back to software if dst has multiple sg entries */
+	if (sg_nents(req->dst) > 1)
 		return deflate_generic_decompress(req);
 
+	if (sg_nents(req->src) > 1) {
+		if (req->slen > PAGE_SIZE)
+			return deflate_generic_decompress(req);
+		use_bounce_src = true;
+	}
+
 	cpu = get_cpu();
 	wq = wq_table_next_wq(cpu);
 	put_cpu();
@@ -1636,20 +1674,45 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 
 	dev = &wq->idxd->pdev->dev;
 
-	if (!dma_map_sg(dev, req->src, 1, DMA_TO_DEVICE)) {
+	if (unlikely(use_bounce_src)) {
+		page = mempool_alloc(iaa_bounce_pool, GFP_ATOMIC);
+		if (!page) {
+			iaa_wq_put(wq);
+			return deflate_generic_decompress(req);
+		}
+
+		if (sg_copy_to_buffer(req->src, sg_nents(req->src),
+				      page_address(page), req->slen) != req->slen) {
+			mempool_free(page, iaa_bounce_pool);
+			iaa_wq_put(wq);
+			return deflate_generic_decompress(req);
+		}
+
+		sg_init_table(&req_ctx->bounce_sg, 1);
+		sg_set_page(&req_ctx->bounce_sg, page, req->slen, 0);
+		req_ctx->bounce_src = page;
+		src = &req_ctx->bounce_sg;
+	}
+
+	if (!dma_map_sg(dev, src, 1, DMA_TO_DEVICE)) {
 		dev_dbg(dev, "couldn't map src sg for iaa device %d, wq %d\n",
 			iaa_wq->iaa_device->idxd->id, iaa_wq->wq->id);
+		if (req_ctx->bounce_src) {
+			mempool_free(req_ctx->bounce_src, iaa_bounce_pool);
+			req_ctx->bounce_src = NULL;
+		}
 		iaa_wq_put(wq);
 		return deflate_generic_decompress(req);
 	}
-	src_addr = sg_dma_address(req->src);
-	dev_dbg(dev, "map src %llx req->src %p slen %d sg_len %d\n", src_addr,
-		req->src, req->slen, sg_dma_len(req->src));
+	src_addr = sg_dma_address(src);
+	dev_dbg(dev, "dma_map_sg, src_addr %llx, src %p,"
+		" req->slen %d, sg_dma_len(sg) %d\n", src_addr,
+		src, req->slen, sg_dma_len(src));
 
 	if (!dma_map_sg(dev, req->dst, 1, DMA_FROM_DEVICE)) {
 		dev_dbg(dev, "couldn't map dst sg for iaa device %d, wq %d\n",
 			iaa_wq->iaa_device->idxd->id, iaa_wq->wq->id);
-		dma_unmap_sg(dev, req->src, 1, DMA_TO_DEVICE);
+		iaa_unmap_src(dev, req);
 		iaa_wq_put(wq);
 		return deflate_generic_decompress(req);
 	}
@@ -1666,7 +1729,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);
 
 	return ret;
@@ -1700,7 +1763,7 @@ static struct acomp_alg iaa_acomp_fixed_deflate = {
 		.cra_driver_name	= "deflate-iaa",
 		.cra_flags		= CRYPTO_ALG_ASYNC,
 		.cra_ctxsize		= sizeof(struct iaa_compression_ctx),
-		.cra_reqsize		= sizeof(u32),
+		.cra_reqsize		= sizeof(struct iaa_req_ctx),
 		.cra_module		= THIS_MODULE,
 		.cra_priority		= IAA_ALG_PRIORITY,
 	}
@@ -1899,6 +1962,12 @@ static int __init iaa_crypto_init_module(void)
 		goto err_aecs_init;
 	}
 
+	iaa_bounce_pool = mempool_create_page_pool(IAA_BOUNCE_POOL_SIZE, 0);
+	if (!iaa_bounce_pool) {
+		ret = -ENOMEM;
+		goto err_bounce_pool;
+	}
+
 	ret = idxd_driver_register(&iaa_crypto_driver);
 	if (ret) {
 		pr_debug("IAA wq sub-driver registration failed\n");
@@ -1932,6 +2001,9 @@ static int __init iaa_crypto_init_module(void)
 err_verify_attr_create:
 	idxd_driver_unregister(&iaa_crypto_driver);
 err_driver_reg:
+	mempool_destroy(iaa_bounce_pool);
+	iaa_bounce_pool = NULL;
+err_bounce_pool:
 	iaa_aecs_cleanup_fixed();
 err_aecs_init:
 
@@ -1948,6 +2020,8 @@ static void __exit iaa_crypto_cleanup_module(void)
 	driver_remove_file(&iaa_crypto_driver.drv,
 			   &driver_attr_verify_compress);
 	idxd_driver_unregister(&iaa_crypto_driver);
+	mempool_destroy(iaa_bounce_pool);
+	iaa_bounce_pool = NULL;
 	iaa_aecs_cleanup_fixed();
 
 	pr_debug("cleaned up\n");

-- 
2.55.0

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Software fallback decompression writes directly to `req->dst` from the CPU while it is still mapped for DMA (`DMA_FROM_DEVICE`), causing silent data corruption if SWIOTLB (bounce buffers) is active.
--

--- Patch [4]: [PATCH 4/4] crypto: iaa - use bounce buffer for multi-sg decompress input ---
commit b4edd2dc0b34db120356af61156eef630f99393a
Author: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

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

This patch introduces a pre-allocated bounce page to linearize small 
multi-entry source scatterlists for the IAA hardware. This allows the 
driver to decompress these requests in hardware instead of falling back 
to software decompression.

> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> index 8f68b1478476e..54bde11c454c9 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c

[ ... ]

> @@ -1124,7 +1147,7 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
>  	}
>  err:
>  	dma_unmap_sg(dev, ctx->req->dst, sg_nents(ctx->req->dst), DMA_FROM_DEVICE);
> -	dma_unmap_sg(dev, ctx->req->src, sg_nents(ctx->req->src), DMA_TO_DEVICE);
> +	iaa_unmap_src(dev, ctx->req);

[Severity: Critical]
This is a pre-existing issue, but does the software fallback mechanism in
iaa_desc_complete() risk silent data corruption if SWIOTLB bounce buffers
are active?

When a decompression request fails in hardware (for example, with an
IAA_ANALYTICS_ERROR), the completion handler falls back to software by
calling deflate_generic_decompress(). This software fallback correctly 
writes the decompressed data into the req->dst scatterlist using the CPU.

However, req->dst is still actively mapped for DMA_FROM_DEVICE. When the
code progresses to this error path and calls dma_unmap_sg() with
DMA_FROM_DEVICE, will SWIOTLB physically copy the contents of the device
bounce buffer (which holds the failed hardware output) over the CPU buffer? 

If so, this would overwrite the successfully decompressed data, which could 
lead to memory corruption if used with subsystems like zswap. Does req->dst 
need to be unmapped prior to invoking deflate_generic_decompress()?

>  out:
>  	if (ret != 0)
>  		dev_dbg(dev, "asynchronous compress failed ret=%d\n", ret);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-iaa-crypto-fixes-zswap-v1-0-65cac23c684d@intel.com?part=4

Re: [PATCH 1/4] dmaengine: idxd: assign all engines to group 0 in IAA defaults

[sashiko-bot]

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

Pre-existing issues:
- [High] After a device FLR (Function Level Reset), group->num_engines and group->num_wqs are not correctly restored, leading to sysfs breakage.
- [High] Double free of `engine`, `wq`, and related structures in initialization error paths and internal cleanup.
- [High] Lockless modifications to engine and wq groups via sysfs expose a TOCTOU NULL pointer dereference in sysfs show functions and cause race conditions on configuration state.
- [High] Unchecked array accesses to `idxd->wqs[0]` and `idxd->groups[0]` when capability registers report 0 for `max_wqs` or `max_groups`.
--

--- Patch [1]: [PATCH 1/4] dmaengine: idxd: assign all engines to group 0 in IAA defaults ---
commit 62dd36d5ea20980d4a54ca9e9b685288c6ec2b65
Author: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Subject: dmaengine: idxd: assign all engines to group 0 in IAA defaults

The IAA device defaults only assigned engine 0 to group 0, leaving engines 1
through max_engines-1 unassigned. This commit assigns all available engines
to group 0 so that the full hardware parallelism is used out of the box
without requiring manual setup.

> diff --git a/drivers/dma/idxd/defaults.c b/drivers/dma/idxd/defaults.c
> index 2bbbcd02a0dac..26ebfa2ca1448 100644
> --- a/drivers/dma/idxd/defaults.c
> +++ b/drivers/dma/idxd/defaults.c
> @@ -8,6 +8,7 @@ int idxd_load_iaa_device_defaults(struct idxd_device *idxd)
>  	struct idxd_engine *engine;
>  	struct idxd_group *group;
>  	struct idxd_wq *wq;
> +	int i;
>  
>  	if (!test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags))
>  		return 0;

[Severity: High]
This is a pre-existing issue, but can this function blindly access idxd->wqs[0]
and idxd->groups[0] if the hardware capability registers report 0 for max_wqs
or max_groups?

If an emulated or malfunctioning device reports num_wqs == 0, the arrays
might be allocated with 0 bytes, returning the ZERO_SIZE_PTR.

drivers/dma/idxd/defaults.c:idxd_load_iaa_device_defaults() {
    ...
    if (!test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags))
        return 0;

    wq = idxd->wqs[0];
    ...
}

Could this dereference the ZERO_SIZE_PTR and cause an out-of-bounds page fault?

[ ... ]

> -	engine = idxd->engines[0];
> -
> -	/* set engine group to 0 */
> -	engine->group = idxd->groups[0];
> -	engine->group->num_engines++;
> +	/* assign all engines to group 0 */
> +	for (i = 0; i < idxd->max_engines; i++) {
> +		engine = idxd->engines[i];
> +		engine->group = group;
> +		group->num_engines++;
> +	}
>  
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does a function level reset fail to properly
restore group->num_engines and group->num_wqs?

When a reset occurs, the state is cleared and the counters are zeroed:

drivers/dma/idxd/device.c:idxd_groups_clear_state() {
    for (i = 0; i < idxd->max_groups; i++) {
        group = idxd->groups[i];
        memset(&group->grpcfg, 0, sizeof(group->grpcfg));
        group->num_engines = 0;
        group->num_wqs = 0;
        ...
    }
}

However, when restoring the configuration, the pointers are restored but the
counters are not updated:

drivers/dma/idxd/init.c:idxd_device_config_restore() {
    for (i = 0; i < idxd->max_engines; i++) {
        struct idxd_engine *saved_engine, *engine;

        saved_engine = idxd_saved->saved_engines[i];
        engine = idxd->engines[i];

        engine->group = saved_engine->group;

        kfree(saved_engine);
    }
}

If an administrator subsequently unassigns an engine via sysfs, could
engine->group->num_engines decrement below zero, resulting in -1 and causing
permanent -EINVAL for future configuration?

[Severity: High]
This is a pre-existing issue, but does this cleanup path double-free the
engine structures?

When initialization fails, put_device() is called which triggers the release
callback:

drivers/dma/idxd/sysfs.c:idxd_conf_engine_release() {
    struct idxd_engine *engine = confdev_to_engine(dev);

    kfree(engine);
}

Immediately after this returns, the cleanup function also frees the engine:

drivers/dma/idxd/init.c:idxd_clean_engines() {
    ...
        conf_dev = engine_confdev(engine);
        put_device(conf_dev);
        kfree(engine);
    ...
}

Can this lead to a use-after-free or double-free corruption?

[Severity: High]
This is a pre-existing issue, but is there a time-of-check to time-of-use race
condition when reading engine groups via sysfs?

The sysfs show function iterates through the engines and performs a lockless
check followed by a dereference:

drivers/dma/idxd/sysfs.c:group_engines_show() {
    ...
        if (!engine->group)
            continue;

        if (engine->group->id == group->id)
    ...
}

Concurrently, the store function modifies this state locklessly:

drivers/dma/idxd/sysfs.c:engine_group_id_store() {
    ...
        if (engine->group) {
            engine->group->num_engines--;
            engine->group = NULL;
        }
    ...
}

If a user writes -1 to unassign an engine while another thread is reading,
could engine->group become NULL after the check but before the dereference,
causing a NULL pointer dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-iaa-crypto-fixes-zswap-v1-0-65cac23c684d@intel.com?part=1

Re: [PATCH 3/4] crypto: iaa - avoid counting fallback decompression bytes

[sashiko-bot]

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

Pre-existing issues:
- [High] Pre-existing DMA API violation: software fallback (`deflate_generic_decompress`) writes to the destination scatterlist while it is still mapped for DMA (`DMA_FROM_DEVICE`), leading to silent data corruption when unmapped if SWIOTLB or confidential computing is active.
--

--- Patch [3]: [PATCH 3/4] crypto: iaa - avoid counting fallback decompression bytes ---
commit 589e7e250edf8839b3b071db5475eb3581825d01
Author: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

crypto: iaa - avoid counting fallback decompression bytes

When decompression falls back to deflate-generic after an analytics
error, the request no longer completes through IAA. Move decompression
byte accounting into the successful IAA completion path in both the
synchronous and asynchronous flows so decomp_bytes only reflects bytes
actually processed by IAA.

> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> index fb154959c2aa8..8f68b1478476e 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> @@ -1084,15 +1084,17 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
>  		}
>  	} else {

[Severity: High]
This is a pre-existing issue, but does calling deflate_generic_decompress()
in the error path just above this block violate the DMA API?

In iaa_desc_complete(), if check_completion() fails with IAA_ANALYTICS_ERROR,
the driver falls back to software decompression:

drivers/crypto/intel/iaa/iaa_crypto_main.c:iaa_desc_complete() {
    ...
    ret = deflate_generic_decompress(ctx->req);
    ...
err:
    dma_unmap_sg(dev, ctx->req->dst, sg_nents(ctx->req->dst), DMA_FROM_DEVICE);
    ...
}

Because the scatterlist is still mapped for DMA, won't dma_unmap_sg()
overwrite the CPU's decompression work with the bounce buffer contents
if SWIOTLB or confidential computing is active?

>  		ctx->req->dlen = idxd_desc->iax_completion->output_size;
> +
> +		if (!ctx->compress) {
> +			update_total_decomp_bytes_in(ctx->req->slen);
> +			update_wq_decomp_bytes(iaa_wq->wq, ctx->req->slen);
> +		}
>  	}

[ ... ]

> @@ -1475,16 +1477,16 @@ static int iaa_decompress(struct crypto_tfm *tfm, struct acomp_req *req,
>  		}
>  	} else {

[Severity: High]
This is a pre-existing issue, but is there a similar DMA API violation here?

In iaa_decompress(), the synchronous path executes the software fallback
and returns to the caller:

drivers/crypto/intel/iaa/iaa_crypto_main.c:iaa_decompress() {
    ...
    ret = deflate_generic_decompress(req);
    ...
out:
    return ret;
}

Does this mean the caller will subsequently unmap the scatterlist and
unknowingly overwrite the fallback's data when SWIOTLB is active?

>  		req->dlen = idxd_desc->iax_completion->output_size;
> +
> +		/* Update stats */
> +		update_total_decomp_bytes_in(slen);
> +		update_wq_decomp_bytes(wq, slen);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-iaa-crypto-fixes-zswap-v1-0-65cac23c684d@intel.com?part=3

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

[sashiko-bot]

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

New issues:
- [High] Hardcoding the `nents` parameter to 1 in `dma_map_sg` causes a kernel panic if `req->dst` is NULL.
- [Medium] The implementation tracks fallback calls rather than processed bytes, contradicting the commit message.

Pre-existing issues:
- [High] Software fallback writes to CPU memory while DMA bounce buffers are mapped, risking data corruption during unmap.
- [High] A timeout during hardware polling causes the CPU to free DMA structures while the hardware device may still be performing DMA, leading to Use-After-Free memory corruption.
--

--- Patch [2]: [PATCH 2/4] crypto: iaa - fall back to software for multi-entry scatterlists ---
commit 6f1532575ac946d82236e0ac6dffd05320f89063
Author: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Subject: crypto: iaa - fall back to software for multi-entry scatterlists

This commit adds a software fallback for the IAA driver when dealing with
scatterlists containing multiple entries, as the hardware cannot process
them directly. It simplifies the DMA mapping path to expect a single entry
and adds counters for the fallback path.

>     Add counters to track the number of bytes processed by the software
>     implementation on the compression direction.

[Severity: Medium]
The commit message states that counters are added to track the number of
bytes processed. However, looking at the updates in iaa_crypto_stats.c:

void update_total_sw_comp_calls(void)
{
        atomic64_inc(&total_sw_comp_calls);
}

Could it be that the implementation only tracks fallback calls rather than
the processed bytes?

> diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> index f62b994e18e58..fb154959c2aa8 100644
> --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
[ ... ]
> @@ -983,17 +984,43 @@ static inline int check_completion(struct device *dev,
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but how does the driver handle hardware
DMA after a timeout?

In check_completion(), if the polling loop exceeds IAA_COMPLETION_TIMEOUT,
it returns -ETIMEDOUT:

check_completion() {
    while (!comp->status) {
        if (status_checks++ >= IAA_COMPLETION_TIMEOUT) {
            ret = -ETIMEDOUT;
            goto out;
        }
        ...
    }
}

When this timeout happens, the caller immediately frees the IDXD descriptor
and unmaps the DMA buffers. However, the hardware might still be actively
processing the request. If the delayed hardware eventually performs a DMA
write to the destination address after the CPU has reallocated that memory
for something else, could this cause silent memory corruption?

> -static int deflate_generic_decompress(struct acomp_req *req)
> +static int deflate_fallback(struct acomp_req *req, bool compress)
[ ... ]
> +static int deflate_generic_decompress(struct acomp_req *req)
> +{
> +	int ret;
> +
> +	ret = deflate_fallback(req, false);
> +	if (ret)
> +		return ret;
> +
>  	update_total_sw_decomp_calls();
>  
> -	return ret;
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem when
deflate_generic_decompress() is called as a fallback from iaa_decompress().

If a hardware error like IAA_ANALYTICS_ERROR occurs in iaa_decompress(),
the software fallback writes to the CPU virtual mapping of req->dst:

iaa_decompress() {
    ...
    if (idxd_desc->iax_completion->status == IAA_ANALYTICS_ERROR) {
        ret = deflate_generic_decompress(req);
        ...
    }
}

Since req->dst is still mapped for DMA_FROM_DEVICE at this point,
when dma_unmap_sg() is eventually called, might it synchronize memory
from a bounce buffer over the original CPU buffer, overwriting the
software fallback's output? Should the scatterlist be unmapped before
invoking the software fallback?

[ ... ]
> @@ -1507,30 +1538,25 @@ static int iaa_comp_acompress(struct acomp_req *req)
[ ... ]
> -	nr_sgs = dma_map_sg(dev, req->dst, sg_nents(req->dst), DMA_FROM_DEVICE);
> -	if (nr_sgs <= 0 || nr_sgs > 1) {
[ ... ]
> +	if (!dma_map_sg(dev, req->dst, 1, DMA_FROM_DEVICE)) {

[Severity: High]
If req->dst is NULL, sg_nents() will return 0, bypassing the fallback check
earlier in this function. Calling dma_map_sg() unconditionally with nents=1
will then attempt to dereference the NULL scatterlist pointer.

Would it be safer to verify req->dst is not NULL before attempting to map
it, preserving the old behavior where sg_nents(req->dst) returning 0 was
handled safely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-iaa-crypto-fixes-zswap-v1-0-65cac23c684d@intel.com?part=2