Re: [PATCH v3 05/19] crypto: cmh - add HMAC ahash
From: sashiko-bot@kernel.org
Date: 2026-08-06 20:09:20
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Unbounded kernel memory leak due to orphaned dynamically allocated chunks when a hash request is aborted before `.final()` or re-initialized/imported. - [Medium] Potential out-of-bounds write in `cmh_hmac_export` when exporting the software fallback state due to missing statesize validation. - [Medium] Mapping an unaligned interior pointer (`chunk->data`) for DMA violates DMA API requirements and risks cacheline corruption or runtime warnings. --
--- Patch [5]: [PATCH v3 05/19] crypto: cmh - add HMAC ahash ---commit 29cce2e66e05b4cda73d07e16e2b43d65f198bd1 Author: Alex Ousherovitch [off-list ref] crypto: cmh - add HMAC ahash This commit registers HMAC-SHA-2 and HMAC-SHA-3 ahash algorithms using the CMH hash core. It implements a self-contained transaction model with software fallback for large accumulated inputs and state cloning.
quoted hunk ↗ jump to hunk
diff --git a/drivers/crypto/cmh/cmh_hmac.c b/drivers/crypto/cmh/cmh_hmac.c new file mode 100644 index 0000000000000..b43b358c35963 --- /dev/null +++ b/drivers/crypto/cmh/cmh_hmac.c@@ -0,0 +1,865 @@
[ ... ]
+struct cmh_hmac_chunk {
+ struct list_head list;
+ struct list_head tfm_node; /* per-tfm orphan tracking */
+ u32 len;
+ u8 data[];
+};
[Severity: Medium]
Does mapping this unaligned data array violate DMA API requirements?
The data array sits at an unaligned offset (36 bytes), which shares a
cacheline with the struct's metadata.
Later, in cmh_hmac_build_sg(), this unaligned interior pointer is passed to
the DMA mapping subsystem:
list_for_each_entry(chunk, &rctx->chunks, list) {
bufs[i].data = chunk->data;
bufs[i].len = chunk->len;
i++;
}
sgm = cmh_dma_build_sg(bufs, rctx->num_chunks, gfp);
Could this risk cacheline corruption on non-coherent architectures and
trigger CONFIG_DMA_API_DEBUG warnings?
[ ... ]+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 cmh_hmac_init() is called on an active request, does this unconditionally overwrite the list head and permanently lose track of any locally buffered chunks? A similar pattern appears in cmh_hmac_import(). [ ... ]
+static int cmh_hmac_update(struct ahash_request *req)
+{[ ... ]
+ chunk = kmalloc(sizeof(*chunk) + req->nbytes,
+ req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP ?
+ GFP_KERNEL : GFP_ATOMIC);
+ if (!chunk) {
+ rctx->error = -ENOMEM;
+ 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]
What happens to these allocations if an unprivileged user creates an
AF_ALG socket, sends data to trigger this allocation, and then closes the
socket before calling recv() to trigger final()?
Since the crypto API has no per-request destructor, and these chunks are
added to the global tctx->all_chunks list, it appears they are only freed
in cmh_hmac_cra_exit():
list_for_each_entry_safe(chunk, tmp, &tctx->all_chunks, tfm_node) {
list_del(&chunk->tfm_node);
kfree_sensitive(chunk);
}
Could a user repeat this in a loop on new accepted sockets to indefinitely
consume kernel memory until the system runs out of memory?
[ ... ]+static int cmh_hmac_export(struct ahash_request *req, void *out)
+{[ ... ]
+ if (rctx->switched) {
+ state->format = CMH_HMAC_FMT_FB;
+ return crypto_ahash_export(cmh_hmac_fb_req(rctx), state->data);
+ }[Severity: Medium] Is there a risk of an out-of-bounds write here if the generic software fallback algorithm allocated in cmh_hmac_cra_init() has a state size larger than CMH_HMAC_EXPORT_MAX? The state buffer size is computed using crypto_ahash_statesize(tfm), but there appears to be no structural validation in cmh_hmac_cra_init() ensuring the allocated fallback's state actually fits within this fixed-size limit before it is exported. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260806195519.2703224-1-skrishnamoorthy@rambus.com?part=5