Re: [PATCH v3 11/19] crypto: cmh - add DRBG hwrng

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning 0 on immediate transient or fatal errors during a blocking read causes a tight CPU loop in the hwrng core.
--

--- Patch [11]: [PATCH v3 11/19] crypto: cmh - add DRBG hwrng ---
commit 8691568ae77b559cf0523f037da202d7654311a0
Author: Alex Ousherovitch <[email protected]>

crypto: cmh - add DRBG hwrng

This commit registers the CMH DRBG core as a hardware RNG provider. It
implements a read callback that submits requests to the hardware
transaction manager.

> diff --git a/drivers/crypto/cmh/cmh_rng.c b/drivers/crypto/cmh/cmh_rng.c
> new file mode 100644
> index 0000000000000..9532b8d66b4c5
> --- /dev/null
> +++ b/drivers/crypto/cmh/cmh_rng.c
[ ... ]
> +static int cmh_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> +{
[ ... ]
> +	ret = cmh_tm_submit_sync_noabort(vcq, DRBG_READ_VCQ_CMDS, 1,
> +					 msecs_to_jiffies(drbg_timeout_ms),
> +					 cmh_dma_orphan_free, orphan);
[ ... ]
> +	if (ret) {
> +		/*
> +		 * .read() only runs with wait=true (see the !wait early
> +		 * return above).  For known transient conditions return 0 so
> +		 * the hwrng core retries the blocking read; a negative errno
> +		 * here would be forwarded to userspace on a blocking fd
> +		 * (e.g. -EAGAIN violates POSIX).  Propagate genuinely
> +		 * unexpected failures so real faults are not masked into an
> +		 * indefinite retry loop.
> +		 */
> +		switch (ret) {
> +		case -EAGAIN:
> +		case -EBUSY:
> +		case -ETIMEDOUT:
> +		case -EIO:
> +		/*
> +		 * -ENODEV: the TM is not running -- occurs when the
> +		 * hwrng kthread (PF_NOFREEZE, not frozen during
> +		 * suspend) calls .read() while the device is suspended.
> +		 * Treat as transient: the TM restarts on resume.
> +		 */
> +		case -ENODEV:
> +			dev_dbg_ratelimited(cmh_dev(),
> +					    "rng: transient DRBG failure (rc=%d)\n",
> +					    ret);
> +			kfree_sensitive(dmabuf);
> +			return 0;

[Severity: High]
If the hardware transaction manager is suspended (-ENODEV) or the command
queue is full (-EAGAIN), cmh_tm_submit_sync_noabort() returns the error
immediately and this code returns 0.

For a blocking read (wait == true), won't the hwrng core receive 0 and loop
without sleeping? This appears to create a tight kernel-space loop that
repeatedly holds and releases the reading_mutex, hogging the CPU until the
transient condition resolves.

> +		default:
> +			dev_err_ratelimited(cmh_dev(),
> +					    "rng: DRBG generate failed (rc=%d)\n",
> +					    ret);
> +			kfree_sensitive(dmabuf);
> +			return ret;
> +		}
> +	}
> +
> +	memcpy(data, dmabuf, nbytes);
> +	kfree_sensitive(dmabuf);
> +
> +	return nbytes;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11