Re: [PATCH v5 07/19] crypto: cmh - add SM3 ahash

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: memory cgroup bypass via unaccounted kmalloc in SM3 driver
--

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

crypto: cmh - add SM3 ahash

This commit registers the SM3 ahash algorithm using the CMH SM3 core.
It supports incremental update, finup, final, and export/import operations.

> diff --git a/drivers/crypto/cmh/cmh_sm3.c b/drivers/crypto/cmh/cmh_sm3.c
> new file mode 100644
> index 0000000000000..a08341512e5d6
> --- /dev/null
> +++ b/drivers/crypto/cmh/cmh_sm3.c
[ ... ]
> +static int cmh_sm3_update(struct ahash_request *req)
> +{
[ ... ]
> +	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.  __GFP_NOWARN
> +	 * keeps a borderline-large (but sub-cap) request quiet if it still
> +	 * cannot be satisfied.
> +	 */
> +	if (full_len > KMALLOC_MAX_SIZE)
> +		return -EMSGSIZE;
> +
> +	rctx->data_buf = kmalloc(full_len, gfp | __GFP_NOWARN);

[Severity: High]
Should this allocation be charged to the caller's memory cgroup by adding the
__GFP_ACCOUNT flag to the gfp mask?

Since the crypto API is accessible to untrusted local users via AF_ALG, an
unprivileged user can submit numerous large asynchronous hash requests. Because
this kmalloc can allocate up to KMALLOC_MAX_SIZE bytes without accounting,
could this allow a malicious container to bypass its memory limits and exhaust
kernel memory, leading to a host-wide out-of-memory condition?

> +	if (!rctx->data_buf)
> +		return -ENOMEM;
[ ... ]
> +static int cmh_sm3_finup(struct ahash_request *req)
> +{
[ ... ]
> +	/* 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: High]
Does this allocation also need __GFP_ACCOUNT for the same reason?

Like the update path above, this allocates unaccounted buffers based on user
input length, which could be abused to bypass memory container limits during
finalization.

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

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