mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev()
@ 2026-10-28  4:03 Li Ximo
  2026-10-28  6:56 ` T Pratham
  0 siblings, 1 reply; 3+ messages in thread
From: Li Ximo @ 2026-10-28  4:03 UTC (permalink / raw)
  To: T Pratham, Herbert Xu, David S. Miller
  Cc: linux-crypto, linux-kernel, Wang Haoran, 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev()
  2026-10-28  4:03 [PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev() Li Ximo
@ 2026-10-28  6:56 ` T Pratham
  2026-11-03  8:47   ` Li Ximo
  0 siblings, 1 reply; 3+ messages in thread
From: T Pratham @ 2026-10-28  6:56 UTC (permalink / raw)
  To: Li Ximo, Herbert Xu, David S. Miller
  Cc: linux-crypto, linux-kernel, Wang Haoran

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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev()
  2026-10-28  6:56 ` T Pratham
@ 2026-11-03  8:47   ` Li Ximo
  0 siblings, 0 replies; 3+ messages in thread
From: Li Ximo @ 2026-11-03  8:47 UTC (permalink / raw)
  To: T Pratham
  Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel,
	Wang Haoran, Li Ximo

Hi Pratham,

On 10/28/26 12:26, T Pratham wrote:
> 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/

Thanks for the pointer - we were not aware of your series. Our patch
duplicates the dthe_get_dev() part of it, and yours goes further by
also fixing the callers, so please consider our patch dropped in favor
of your series. We will review your patches instead.

Sorry for the noise, and thanks for the heads-up.

Best regards,
Li Ximo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-03  8:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28  4:03 [PATCH] crypto: ti - dthev2: fix empty-list wild pointer in dthe_get_dev() Li Ximo
2026-10-28  6:56 ` T Pratham
2026-11-03  8:47   ` Li Ximo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®