Fwd: [PATCH v22 3/4] crypto: spacc - add spacc autodetect support
Pavitrakumar Managutte <[email protected]>
| Newsgroups | org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CALxtO0kvQp333oRZ5axaSG1-8y7E9T_q5zQX6RcsFwQb=_rdBw@mail.gmail.com> |
Hi Herbert, I need your input on the issue below in the SPAcc driver related to the DDT (scatter-gather) DMA. I detailed the issue in the inline comment below. Warm regards, PK ---------- Forwarded message --------- From: <[email protected]> Date: Wed, Aug 12, 2026 at 11:35 PM Subject: Re: [PATCH v22 3/4] crypto: spacc - add spacc autodetect support To: Pavitrakumar Managutte <[email protected]> Cc: <[email protected]>, <[email protected]>, <[email protected]> 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? PK: SPAcc DDT(scatter-gather) DMA has a design limitation: we can't kill or reset an in-flight DDT DMA operation programmatically. Once a DDT DMA operation begins, it runs-to-completion with either - successful DMA transfer or - errors out with proper error codes Sashiko bot review has problem with both ("wait_event" & "wait_event_timeout" usage) in code. 1. With "wait_event" the bot raises hang issue. 2. With "wait_event_timeout" it raises the 'no option to stop in-flight DMA operation' issue. I have tried both options, and the driver works perfectly fine, but Sashiko bot flags issues with both variations. I need your input to resolve this, as I'm stuck with this Sashiko bot issue. I prefer "wait_event" since I want the DMA operation to run-to-completion. Please suggest. > + > + job->job_done = 0; > + err = job->job_err; > + } [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3