Re: [PATCH v22 3/4] crypto: spacc - add spacc autodetect support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unkillable wait without a timeout in hardware autodetection can permanently hang the kernel.
--

--- Patch [3]: [PATCH v22 3/4] crypto: spacc - add spacc autodetect support ---
commit 1b02ffd964af3b1927d2fca1ccd5966f8d928e2e
Author: Pavitrakumar Managutte <[email protected]>
crypto: spacc - add spacc autodetect support

This commit adds autodetection support to the SPAcc driver to determine
supported cryptographic algorithms. It runs test vectors through the hardware
during initialization and disables algorithms that fail to produce the
expected output.

> diff --git a/drivers/crypto/dwc-spacc/spacc_core.c b/drivers/crypto/dwc-spacc/spacc_core.c
> index b72d5072ccc58..3287e9e61b1c0 100644
> --- a/drivers/crypto/dwc-spacc/spacc_core.c
> +++ b/drivers/crypto/dwc-spacc/spacc_core.c
[ ... ]
> @@ -579,6 +1480,275 @@ int spacc_close(struct spacc_device *dev, int handle)
[ ... ]
> +#if IS_ENABLED(CONFIG_CRYPTO_DEV_SPACC_AUTODETECT)
[ ... ]
> +static int spacc_check_modes(struct spacc_device *spacc, int algo_mode,
> +			      int keysz_idx, void *virt, char *key,
> +			      struct pdu_ddt *ddt)
> +{
[ ... ]
> +	err = spacc_packet_enqueue_ddt(spacc, rc, ddt, ddt, proclen, 0,
> +				       aadlen, 0, 0, 0);
> +	job = &spacc->job[rc];
> +
> +	if (err == 0) {
> +		/*
> +		 * No timeout, not interruptible: this only returns once
> +		 * job_done is actually true, i.e. the hardware has
> +		 * genuinely relinquished the job. That guarantees it is
> +		 * always safe to close the job/context and (via the
> +		 * caller) free the DMA buffer immediately below --
> +		 * there is no "unconfirmed" case left to handle.
> +		 *
> +		 * Tradeoff: if the hardware ever genuinely fails to
> +		 * complete a submitted job, this hangs here
> +		 * unkillably (TASK_UNINTERRUPTIBLE, no timeout) for the
> +		 * rest of probe. Accepted deliberately in place of the
> +		 * abandoned-job/UAF risk of giving up early with no
> +		 * hardware cancel available.
> +		 */
> +		wait_event(job->waitq, job->job_done);

[Severity: High]
Can this wait_event() cause a permanent kernel thread hang?

If the hardware fails to respond or trigger the completion interrupt, this
uninterruptible wait will never complete. If the driver is built-in and
probes synchronously, couldn't this stall the entire system boot process,
or otherwise result in hung task panics?

While the comment notes this as a deliberate tradeoff to avoid DMA
use-after-free risks upon abandoning the job, is it possible to implement a
safe hardware cancellation or reset path and use wait_event_timeout()
instead of relying on an unkillable wait?

> +
> +		job->job_done = 0;
> +		err = job->job_err;
> +	}
[ ... ]

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