Re: [PATCH v5 04/19] crypto: cmh - add SHA-2/SHA-3/SHAKE ahash

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] crypto: cmh: ahash rejects valid large scatterlists
--

commit 75a869d1380414cdb3e2f319fd8862f445b10395
Author: Alex Ousherovitch <aousherovitch@rambus.com>
Date:   Thu Sep 17 22:59:13 2026 +0000

    crypto: cmh - add SHA-2/SHA-3/SHAKE ahash

This commit registers ahash algorithms for the SHA-2, SHA-3, and SHAKE
families using the CMH hash core. It implements incremental update/finup and
export/import for request cloning, relying on the crypto API's partial block
buffering.

> diff --git a/drivers/crypto/cmh/cmh_hash.c b/drivers/crypto/cmh/cmh_hash.c
> new file mode 100644
> index 0000000000000..1acc8bdab3522
> --- /dev/null
> +++ b/drivers/crypto/cmh/cmh_hash.c
[ ... ]
> +static int cmh_hash_update(struct ahash_request *req)
> +{
> +	struct cmh_hash_reqctx *rctx = ahash_request_ctx(req);
> +	const struct cmh_hash_alg_info *info = rctx->info;
> +	struct vcq_cmd cmds[CMH_HASH_MAX_PAYLOAD];
> +	struct core_dispatch d;
> +	u32 block_size = info->block_size;
> +	u32 full_len;
> +	u32 idx;
> +	int ret;
> +	gfp_t gfp;
> +
> +	if (rctx->error)
> +		return rctx->error;
> +
> +	if (!req->nbytes)
> +		return 0;
> +
> +	/*
> +	 * block_size is not always a power of two (SHA-3 rates: 144/136/
> +	 * 104/72), so use modulo -- round_down() would corrupt the split.
> +	 */
> +	rctx->update_remainder = req->nbytes % block_size;
> +	full_len = req->nbytes - rctx->update_remainder;
> +
> +	gfp = req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP ?
> +	      GFP_KERNEL : GFP_ATOMIC;
> +
> +	/*
> +	 * Reject a single update whose linearisation would exceed the largest
> +	 * kmalloc: return a permanent -EMSGSIZE ("message too long") rather
> +	 * than a transient -ENOMEM the client would keep retrying.
> +	 */
> +	if (full_len > KMALLOC_MAX_SIZE)
> +		return -EMSGSIZE;
> +
> +	/*
> +	 * Linearise the block-aligned prefix from the scatterlist.
> +	 * __GFP_NOWARN keeps a borderline-large (but sub-cap) request from
> +	 * splatting the page allocator if it still cannot be satisfied.
> +	 */
> +	rctx->data_buf = kmalloc(full_len, gfp | __GFP_NOWARN);

[Severity: Medium]
Since the Linux crypto API contract permits arbitrary-sized inputs via
scatterlists, is it okay to impose an allocation limit here?

A caller submitting an update with a large nbytes could be rejected with
-EMSGSIZE or fail with -ENOMEM due to physical memory fragmentation. Could
this code process the scatterlist in smaller chunks instead of attempting
to linearise the entire payload into a single physically contiguous buffer?

[ ... ]
> +static int cmh_hash_finup(struct ahash_request *req)
> +{
> +	struct cmh_hash_reqctx *rctx = ahash_request_ctx(req);
> +	u32 data_len = req->nbytes;
> +	u8 *data_buf = NULL;
> +	gfp_t gfp;
> +
> +	if (rctx->error)
> +		return rctx->error;
> +
> +	if (data_len == 0)
> +		return cmh_hash_submit_final(req, NULL, 0);
> +
> +	/* Reject an oversized linearisation with a permanent -EMSGSIZE. */
> +	if (data_len > KMALLOC_MAX_SIZE)
> +		return -EMSGSIZE;
> +
> +	gfp = req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP ?
> +	      GFP_KERNEL : GFP_ATOMIC;
> +
> +	data_buf = kmalloc(data_len, gfp | __GFP_NOWARN);

[Severity: Medium]
Does this code also need to process the scatterlist in chunks?

Similar to cmh_hash_update(), attempting to allocate a contiguous buffer for
the entire finalization payload restricts the maximum input size and violates
the ahash scatterlist API contract.

> +	if (!data_buf)
> +		return -ENOMEM;
> +
> +	scatterwalk_map_and_copy(data_buf, req->src, 0, data_len, 0);
> +
> +	return cmh_hash_submit_final(req, data_buf, data_len);
> +}
> +

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