Re: [PATCH v23 3/4] crypto: spacc - add spacc autodetect support
From: sashiko-bot@kernel.org
Date: 2026-08-27 04:55:11
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Use of `wait_event_killable` contradicts safety assumptions, allowing fatal signals to cause DMA Use-After-Free -- commit e7d03dccc593f00d608b75c3b090d8bba8e22258 Author: Pavitrakumar Managutte [off-list ref] crypto: spacc - add spacc autodetect support This commit adds autodetection support to the SPAcc crypto driver. It dynamically tests which algorithms the hardware supports by issuing test operations against the hardware during initialization.
quoted hunk ↗ jump to hunk
diff --git a/drivers/crypto/dwc-spacc/spacc_core.c b/drivers/crypto/dwc-spacc/spacc_core.c --- a/drivers/crypto/dwc-spacc/spacc_core.c +++ b/drivers/crypto/dwc-spacc/spacc_core.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -579,6 +1480,276 @@ int spacc_close(struct spacc_device *dev, int handle)
[ ... ]
+static int spacc_check_modes(struct spacc_device *spacc, int algo_mode,
+ int keysz_idx, void *virt, char *key,
+ struct pdu_ddt *ddt)
+{[ ... ]
+ 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_killable(job->waitq, job->job_done);[Severity: High] The comment explicitly states this wait should be "not interruptible" and uses TASK_UNINTERRUPTIBLE to prevent abandoned-job and use-after-free risks. However, wait_event_killable() can be interrupted by a fatal signal. If a fatal signal (like SIGKILL) is delivered, won't this return early without checking the return value? Since the hardware is still processing, does returning here allow the caller to prematurely free the DMA buffers?
+ + job->job_done = 0; + err = job->job_err; + }
[ ... ]
+int spacc_autodetect(struct spacc_device *spacc)
+{[ ... ]
+ for (y = 0; y < ARRAY_SIZE(keysizes[0]); y++) {
+ /*
+ * spacc_check_modes() now always returns 0 --
+ * it only comes back once the job is confirmed
+ * done, closes it itself, and disables the mode
+ * bit internally on any failure/mismatch. Nothing
+ * left for this loop to branch on.
+ */
+ spacc_check_modes(spacc, x, y, virt, key, &ddt);
+ }
+
+ if (!(spacc->config.modes[x] & 0x7F))
+ spacc->config.modes[x] &= ~128;
+ }
+
+ pdu_ddt_free(&ddt);
+ dma_free_coherent(spacc->dptr, SPACC_TEST_DMA_BUFF_SIZE, virt, dma);[Severity: High] Following the potential early return from wait_event_killable() in spacc_check_modes() above, these DMA buffers will be freed here. Will this cause a use-after-free and DMA corruption when the hardware eventually completes the abandoned job and writes to this freed memory?
+ spacc->autodetect = false; + + return 0; +}
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260827044349.826534-1-pavitrakumarm@vayavyalabs.com?part=3