Re: [PATCH v4 05/19] crypto: cmh - add HMAC ahash

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded kernel memory leak via abandoned or re-initialized AF_ALG requests due to chunks orphaned in a shared TFM list.
--

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

crypto: cmh - add HMAC ahash

This commit registers HMAC ahash algorithms for the CMH hash core,
supporting HMAC-SHA-2 and HMAC-SHA-3. It implements a software
accumulation buffer to handle intermediate HMAC state.

> diff --git a/drivers/crypto/cmh/cmh_hmac.c b/drivers/crypto/cmh/cmh_hmac.c
> new file mode 100644
> index 0000000000000..528079f60bfa9
> --- /dev/null
> +++ b/drivers/crypto/cmh/cmh_hmac.c
[ ... ]
> +static int cmh_hmac_init(struct ahash_request *req)
> +{
> +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> +	struct cmh_hmac_reqctx *rctx = ahash_request_ctx(req);
> +
> +	rctx->info = cmh_hmac_get_info(tfm);
> +	rctx->error = 0;
> +	INIT_LIST_HEAD(&rctx->chunks);

[Severity: High]
If an active request is re-initialized by calling cmh_hmac_init(), does this
INIT_LIST_HEAD() blindly wipe the list head and permanently sever access to
previously accumulated chunks, leaving them stranded on the TFM list?

[ ... ]
> +static int cmh_hmac_update(struct ahash_request *req)
> +{
[ ... ]
> +	chunk->len = req->nbytes;
> +	if (req->base.flags & CRYPTO_AHASH_REQ_VIRT) {
> +		memcpy(chunk->data, req->svirt, req->nbytes);
> +	} else {
> +		nents = sg_nents_for_len(req->src, req->nbytes);
> +		if (nents < 0 ||
> +		    sg_copy_to_buffer(req->src, nents,
> +				      chunk->data, req->nbytes) != req->nbytes) {
> +			kfree_sensitive(chunk);
> +			rctx->error = -EINVAL;
> +			goto err_free_chunks;
> +		}
> +	}
> +
> +	list_add_tail(&chunk->list, &rctx->chunks);
> +	spin_lock_bh(&tctx->chunk_lock);
> +	list_add_tail(&chunk->tfm_node, &tctx->all_chunks);
> +	spin_unlock_bh(&tctx->chunk_lock);

[Severity: High]
Since chunks are added to the per-transform list to survive request
destruction, doesn't this tie their lifecycle to the long-lived TFM?

Could this cause an unbounded kernel memory leak? An unprivileged local user
can create an AF_ALG socket, call accept() to create child sockets sharing
the same parent TFM, send data to allocate internal chunks here, and then
close the child socket. Since the crypto API lacks a per-request destructor
hook, the request context is freed without notifying the driver, leaving the
chunks orphaned on the TFM list.

[ ... ]
> +static int cmh_hmac_import(struct ahash_request *req, const void *in)
> +{
[ ... ]
> +	/*
> +	 * Do NOT call free_chunks() here: the crypto API does not
> +	 * guarantee the request context is in a valid state before
> +	 * import(), so the list pointers may be stale or invalid.
> +	 * Re-initialize from scratch instead.  Any pre-existing chunks
> +	 * are tracked on tctx->all_chunks and freed in cra_exit.
> +	 */
> +	rctx->info = cmh_hmac_get_info(tfm);
> +	rctx->error = 0;
> +	INIT_LIST_HEAD(&rctx->chunks);

[Severity: High]
Similar to cmh_hmac_init(), does importing over an active request orphan its
chunks? The comment acknowledges reliance on cmh_hmac_cra_exit() for cleanup,
which confirms the memory remains tied up for the lifetime of the TFM.

[ ... ]
> +static void cmh_hmac_cra_exit(struct crypto_tfm *tfm)
> +{
> +	struct cmh_hmac_tfm_ctx *tctx = crypto_tfm_ctx(tfm);
> +	struct cmh_hmac_chunk *chunk, *tmp;
> +
> +	/* Free any orphaned chunks (e.g. testmgr export/reimport poison) */
> +	spin_lock_bh(&tctx->chunk_lock);
> +	list_for_each_entry_safe(chunk, tmp, &tctx->all_chunks, tfm_node) {
> +		list_del(&chunk->tfm_node);
> +		kfree_sensitive(chunk);
> +	}
> +	spin_unlock_bh(&tctx->chunk_lock);

[Severity: High]
Sweeping orphaned chunks only at TFM destruction causes unbounded
accumulation when many requests are created and destroyed over the lifetime
of a single TFM. Is there a way to manage chunk lifetimes without tying them
to the TFM?

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