Re: [PATCH 4/5] crypto: cmh - add AES skcipher/aead/cmac

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: spurious ENOMEM on crypto requests due to high-order kmalloc
--

commit 6acd06ca4d876414812511190b70a583e7f5687b
Author: Alex Ousherovitch <aousherovitch@rambus.com>

crypto: cmh - add AES skcipher/aead/cmac

This commit registers AES algorithms using the CMH AES core for skcipher, 
aead, and ahash. It supports 128, 192, and 256-bit keys, and routes
operations through hardware acceleration.

> diff --git a/drivers/crypto/cmh/cmh_aes.c b/drivers/crypto/cmh/cmh_aes.c
> new file mode 100644
> index 0000000000000..d4352caf771f6
> --- /dev/null
> +++ b/drivers/crypto/cmh/cmh_aes.c
[ ... ]
> @@ -371,19 +474,16 @@ static int cmh_aes_crypt(struct skcipher_request *req, u32 aes_op)
> +	/*
> +	 * Linearise input from scatterlist.  cryptlen is user-controlled up
> +	 * to CMH_AES_MAX_CRYPTLEN (well above KMALLOC_MAX_SIZE), so use
> +	 * __GFP_NOWARN: an oversized request fails cleanly with -ENOMEM
> +	 * instead of splatting the page allocator.
> +	 */
> +	rctx->in_buf = kmalloc(req->cryptlen, gfp | __GFP_NOWARN);
> +	if (!rctx->in_buf)
> +		return -ENOMEM;
> +
> +	scatterwalk_map_and_copy(rctx->in_buf, req->src, 0, req->cryptlen, 0);
> +
> +	rctx->in_dma = cmh_dma_map_single(rctx->in_buf, req->cryptlen,
> +					  DMA_TO_DEVICE);
> +	if (cmh_dma_map_error(rctx->in_dma)) {
> +		ret = -ENOMEM;
> +		goto out_free_in;
> +	}
> +
> +	/* Allocate and map output buffer */
> +	rctx->out_buf = kmalloc(req->cryptlen, gfp | __GFP_NOWARN);
> +	if (!rctx->out_buf) {
> +		ret = -ENOMEM;
> +		goto out_unmap_in;
> +	}

[Severity: High]
Will this contiguous allocation fail under normal system memory fragmentation? 

The skcipher cryptlen can be up to CMH_AES_MAX_CRYPTLEN (32MB), but kmalloc() 
has a hard limit of KMALLOC_MAX_SIZE (typically 4-8MB) and will 
deterministically fail for larger sizes. Even for moderately large sizes 
(e.g. 64KB - 1MB), kmalloc() is highly prone to failing when system physical 
memory fragments.

Because the crypto framework does not automatically retry with a software 
fallback when the hardware driver returns -ENOMEM, valid requests (such as 
those from dm-crypt or IPsec) might unexpectedly fail and break caller 
functionality. Could this be changed to support scatter-gather directly or 
use a fallback allocation strategy if the hardware supports it?

> diff --git a/drivers/crypto/cmh/cmh_aes_aead.c b/drivers/crypto/cmh/cmh_aes_aead.c
> new file mode 100644
> index 0000000000000..ed2a0b9449bc0
> --- /dev/null
> +++ b/drivers/crypto/cmh/cmh_aes_aead.c
[ ... ]
> @@ -622,15 +740,11 @@ static int cmh_aes_aead_crypt(struct aead_request *req, u32 aes_op)
> +	/* Linearise input */
> +	if (cryptlen > 0) {
> +		rctx->in_buf = kmalloc(cryptlen, gfp | __GFP_NOWARN);
> +		if (!rctx->in_buf) {
> +			ret = -ENOMEM;
> +			goto out_unmap_aad;
> +		}

[Severity: High]
Similarly here in cmh_aes_aead_crypt(), the driver attempts to allocate up to
1MB of contiguous memory using kmalloc(). Does this run the same risk of
failing due to memory fragmentation and propagating spurious -ENOMEM errors
to the AEAD callers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013174847.2192609-1-aousherovitch@rambus.com?part=4