[PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev()

[PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev()

[Li Ximo]

From: Wang Haoran <haoranwangsec@gmail.com>

While auditing list_first_entry() users we found that dthe_get_dev()
calls list_first_entry() on the global dthe_dev_list.dev_list without
checking whether the list is empty. list_first_entry() never returns
NULL: on an empty list head->next == head, so container_of() produces
a bogus non-NULL pointer into the static dthe_dev_list structure. The
existing "if (dev_data)" check is therefore dead code - it can never
be false - and list_move_tail(&dev_data->list, ...) then reads and
writes through that wild pointer, corrupting the list head, after
which callers use the bogus pointer for engine/MMIO accesses.

The empty list is reachable because dthe_remove() removes the device
from the list (list_del) before unregistering the crypto algorithms
(dthe_unregister_algs). On single-device SoCs the list is empty from
that point on, while the algorithms remain registered, so a new
tfm (ctx->dev_data not yet cached) can still reach dthe_get_dev() and
perform the lookup on the empty list. K3 DTHE is a platform device, so
unbind/remove is enough to open this window.

Fix this with two changes: switch dthe_get_dev() to
list_first_entry_or_null(), which makes the existing NULL check
effective and returns NULL when no device is registered; and reorder
dthe_remove() to unregister the algorithms before removing the device
from the list, mirroring the probe order (list_add before
dthe_register_algs) and preventing new requests from being dispatched
once the device is gone. This was found by code audit; we do not have
K3 hardware to reproduce it.

Note that the nine dthe_get_dev() call sites in dthev2-aes.c do not
check for a NULL return. With the remove() reordering the empty-list
window is closed, so the NULL return is purely defensive; adding NULL
checks to the callers may nevertheless be a worthwhile follow-up.

Reported-by: Wang Haoran <haoranwangsec@gmail.com>
Reported-by: Li Ximo <lixicaicai@gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
---
 drivers/crypto/ti/dthev2-common.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ti/dthev2-common.c b/drivers/crypto/ti/dthev2-common.c
index ba6cde75d..099c0b923 100644
--- a/drivers/crypto/ti/dthev2-common.c
+++ b/drivers/crypto/ti/dthev2-common.c
@@ -39,7 +39,8 @@ struct dthe_data *dthe_get_dev(struct dthe_tfm_ctx *ctx)
 		return ctx->dev_data;
 
 	spin_lock_bh(&dthe_dev_list.lock);
-	dev_data = list_first_entry(&dthe_dev_list.dev_list, struct dthe_data, list);
+	dev_data = list_first_entry_or_null(&dthe_dev_list.dev_list,
+					    struct dthe_data, list);
 	if (dev_data)
 		list_move_tail(&dev_data->list, &dthe_dev_list.dev_list);
 	spin_unlock_bh(&dthe_dev_list.lock);
@@ -200,12 +201,12 @@ static void dthe_remove(struct platform_device *pdev)
 {
 	struct dthe_data *dev_data = platform_get_drvdata(pdev);
 
+	dthe_unregister_algs();
+
 	spin_lock(&dthe_dev_list.lock);
 	list_del(&dev_data->list);
 	spin_unlock(&dthe_dev_list.lock);
 
-	dthe_unregister_algs();
-
 	crypto_engine_exit(dev_data->engine);
 
 	dma_release_channel(dev_data->dma_aes_rx);
-- 
2.30.2

Re: [PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev()

[T Pratham]

On 10/28/26 09:33, Li Ximo wrote:
> From: Wang Haoran <haoranwangsec@gmail.com>
> 
> While auditing list_first_entry() users we found that dthe_get_dev()
> calls list_first_entry() on the global dthe_dev_list.dev_list without
> checking whether the list is empty. list_first_entry() never returns
> NULL: on an empty list head->next == head, so container_of() produces
> a bogus non-NULL pointer into the static dthe_dev_list structure. The
> existing "if (dev_data)" check is therefore dead code - it can never
> be false - and list_move_tail(&dev_data->list, ...) then reads and
> writes through that wild pointer, corrupting the list head, after
> which callers use the bogus pointer for engine/MMIO accesses.
> 
> The empty list is reachable because dthe_remove() removes the device
> from the list (list_del) before unregistering the crypto algorithms
> (dthe_unregister_algs). On single-device SoCs the list is empty from
> that point on, while the algorithms remain registered, so a new
> tfm (ctx->dev_data not yet cached) can still reach dthe_get_dev() and
> perform the lookup on the empty list. K3 DTHE is a platform device, so
> unbind/remove is enough to open this window.
> 
> Fix this with two changes: switch dthe_get_dev() to
> list_first_entry_or_null(), which makes the existing NULL check
> effective and returns NULL when no device is registered; and reorder
> dthe_remove() to unregister the algorithms before removing the device
> from the list, mirroring the probe order (list_add before
> dthe_register_algs) and preventing new requests from being dispatched
> once the device is gone. This was found by code audit; we do not have
> K3 hardware to reproduce it.
> 
> Note that the nine dthe_get_dev() call sites in dthev2-aes.c do not
> check for a NULL return. With the remove() reordering the empty-list
> window is closed, so the NULL return is purely defensive; adding NULL
> checks to the callers may nevertheless be a worthwhile follow-up.
> 
> Reported-by: Wang Haoran <haoranwangsec@gmail.com>
> Reported-by: Li Ximo <lixicaicai@gmail.com>
> Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
> ---

Hi Wang

Thanks for the patch. However, this fix is already in works in this series:

[PATCH v10 00/15] Fix several issues in DTHEv2 driver
https://lore.kernel.org/all/20261016124456.1802080-1-t-pratham@ti.com/

Specifically this is the patch:
https://lore.kernel.org/all/20261016124456.1802080-2-t-pratham@ti.com/

-- 
Regards
T Pratham <t-pratham@ti.com>