Re: [PATCH v6 9/9] crypto: atmel: Use dmaengine_prep_config_sg() API

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Tasklets can be rescheduled after they are killed during device removal, leading to a Use-After-Free.
- [High] The device unbind path does not wait for active crypto transforms (TFMs) to be freed, leading to a Use-After-Free.
- [High] Pending requests in the crypto queue are not completed or flushed during device removal.
- [High] Calling `dmaengine_terminate_sync()` from atomic context (tasklet/softirq) in the error path.
--

commit 8fb292764f2c30a42b59a7ee1bce9dcae3cc201f
Author: Frank Li <Frank.Li@nxp.com>

crypto: atmel: Use dmaengine_prep_config_sg() API

This commit simplifies the driver by replacing dmaengine_slave_config()
and dmaengine_prep_slave_sg() with the combined dmaengine_prep_config_sg()
API. It converts both configuration and descriptor preparation failures to
return -ENOMEM.

> diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
> index b393689400b4c..d890b5a277b9c 100644
> --- a/drivers/crypto/atmel-aes.c
> +++ b/drivers/crypto/atmel-aes.c
> @@ -795,7 +795,6 @@ static int atmel_aes_dma_transfer_start(struct atmel_aes_dev *dd,
[ ... ]
> -	err = dmaengine_slave_config(dma->chan, &config);
> -	if (err)
> -		return err;
> -
> -	desc = dmaengine_prep_slave_sg(dma->chan, dma->sg, dma->sg_len, dir,
> -				       DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	desc = dmaengine_prep_config_sg(dma->chan, dma->sg, dma->sg_len, dir,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK,
> +					&config);
>  	if (!desc)
>  		return -ENOMEM;

[Severity: High]
This is a pre-existing issue, but can this error path lead to a kernel BUG
by sleeping in an atomic context?

If dmaengine_prep_config_sg() fails and returns NULL, it seems the failure
propagates up to atmel_aes_dma_start() which might jump to an error label
and call dmaengine_terminate_sync().

Since the crypto request queue is processed via atmel_aes_queue_task()
(a tasklet running in softirq context), wouldn't calling
dmaengine_terminate_sync() (which uses might_sleep()) from this atomic
context cause issues?

[Severity: High]
This is a pre-existing issue, but does device removal have a use-after-free
vulnerability if a DMA transfer completes concurrently?

In atmel_aes_remove(), tasklet_kill() is called before
atmel_aes_dma_cleanup(). If a DMA transfer completes immediately after
tasklet_kill(), the interrupt callback (atmel_aes_dma_callback()) could
execute and schedule the queue_task tasklet again via tasklet_schedule().

Since the aes_dd driver data structure is managed via devm_kzalloc() and
freed upon return from atmel_aes_remove(), would the tasklet subsequently
access freed memory when it runs?

[Severity: High]
This is a pre-existing issue, but does the unbind path properly wait for
active crypto transforms (TFMs) to be freed?

In atmel_aes_remove(), the driver unregisters its algorithms using
crypto_unregister_algs(). This prevents new TFMs from being instantiated,
but it doesn't appear to revoke or wait for active TFMs to be freed.

Because aes_dd is allocated using devm_kzalloc() and freed once
atmel_aes_remove() completes, could subsequent crypto operations triggered
by userspace via remaining TFMs dereference the freed aes_dd structure?

[Severity: High]
This is a pre-existing issue, but are pending requests in the crypto queue
completed or flushed during device removal?

During device unbind and atmel_aes_remove(), it appears the driver never
purges, aborts, or flushes the crypto requests currently waiting in
aes_dd->queue.

When the device structure is freed by devres, wouldn't all queued
asynchronous requests be lost without invoking their completion callbacks,
causing callers waiting on wait_for_completion() to hang indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-dma_prep_config-v6-0-06e49b7acb38@nxp.com?part=9