Re: [PATCH v23 09/14] crypto: qce - Remove unused ignore_buf

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The DMA channels and `result_buf` are released before the asynchronous request workqueue (`done_work`) is cancelled, leading to a Use-After-Free of DMA channels and buffer during driver unbind.
--

--- Patch [9]: [PATCH v23 09/14] crypto: qce - Remove unused ignore_buf ---
commit c6ae497c31e39eb0af424aa124dcc9493834ad11
Author: Bartosz Golaszewski <brgl@kernel.org>

crypto: qce - Remove unused ignore_buf
    
This commit removes the unused ignore_buf field from the qce_dma_data
structure. It also cleans up the associated offset calculation in the
DMA request initialization.

> diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
> index 7ec9d72fd690f..d1daa229361aa 100644
> --- a/drivers/crypto/qce/dma.c
> +++ b/drivers/crypto/qce/dma.c

[ ... ]

> @@ -43,8 +45,6 @@ int devm_qce_dma_request(struct device *dev, struct qce_dma_data *dma)
>  		goto error_nomem;
>  	}
>  
> -	dma->ignore_buf = dma->result_buf + QCE_RESULT_BUF_SZ;
> -
>  	return devm_add_action_or_reset(dev, qce_dma_release, dma);

[Severity: High]
This is a pre-existing issue, but registering qce_dma_release() here creates
a teardown order problem with the asynchronous request workqueue.

In drivers/crypto/qce/core.c, qce_crypto_probe() registers the cleanup action
for qce->done_work before this DMA request:

qce_crypto_probe() {
    INIT_WORK(&qce->done_work, qce_req_done_work);
    devm_add_action_or_reset(dev, qce_cancel_work, &qce->done_work);
    ...
    devm_qce_dma_request(qce->dev, &qce->dma);
}

Because devres uses a LIFO execution order during unbind, qce_dma_release()
will execute before the workqueue is canceled. 

If a user unbinds the driver while crypto operations are ongoing,
qce_dma_release() will free the DMA channels and dma->result_buf. If a DMA
completion interrupt had already scheduled qce->done_work, that work sits
pending. 

When qce_cancel_work() finally runs, cancel_work_sync() will flush the work,
allowing qce_req_done_work() to call qce_handle_queue() and attempt to submit
the next request using the already-freed dma->result_buf and DMA channels.

Could the workqueue cleanup be registered after the DMA request to ensure 
done_work is explicitly canceled before the DMA channels and buffers are 
released?

>  
>  error_nomem:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-qcom-qce-cmd-descr-v23-0-1be02deffda9@oss.qualcomm.com?part=9