linux-kernel - Re: [PATCH] tee: fix illegal pointer dereference in tee_shm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHUa44FAUyQNBKmzugu-_gv_Jy_AftZqq=RSbKUnK1QQbL8Z9A@mail.gmail.com>
Date: Thu, 6 Nov 2025 12:31:10 +0100
From: Jens Wiklander <jens.wiklander@...aro.org>
To: yangzhao <yangzhao@...inos.cn>
Cc: op-tee@...ts.trustedfirmware.org, linux-kernel@...r.kernel.org, 
	Sumit Garg <sumit.garg@...nel.org>
Subject: Re: [PATCH] tee: fix illegal pointer dereference in tee_shm_put()

Hi,

On Thu, Nov 6, 2025 at 11:20 AM yangzhao <yangzhao@...inos.cn> wrote:
>
> In tee_shm_put(), there is not only the NULL pointer dereference,
> but also the illegal pointer dereference.
>
> shutdown() --->
>     __optee_disable_shm_cache -->
>         shm = reg_pair_to_ptr(...);
>         tee_shm_free(shm); -->
>             tee_shm_put(shm); //crash: shm->ctx maybe NULL pointer or illegal pointer
>
> Check whether the pointer is NULL and whether the pointer address is valid.
>
> This issue occurs when rich world uses the 6.x version of the kernel and
> optee secure world uses a lower version (such as version 3.2), and it is
> highly likely to trigger a kernel panic when conducting hibernate tests.

It sounds like the root of the problem is in the TEE rather than in
the kernel. How about fixing the TEE to avoid supplying garbage
pointers?

>
> Fixes: e4a718a3a47e ("tee: fix NULL pointer dereference in tee_shm_put")
> Signed-off-by: yangzhao <yangzhao@...inos.cn>
> ---
>  drivers/tee/tee_shm.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 4a47de4bb2e5..de01d16409c1 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -722,7 +722,14 @@ void tee_shm_put(struct tee_shm *shm)
>         struct tee_device *teedev;
>         bool do_release = false;
>
> -       if (!shm || !shm->ctx || !shm->ctx->teedev)
> +       /* checking pointer */
> +       if (IS_ERR_OR_NULL(shm) || !virt_addr_valid(shm))

The IS_ERR_OR_NULL() check might make sense, but the virt_addr_valid()
does not. virt_addr_valid() might catch a few garbage pointers, but
the real problem is that someone is supplying garbage pointers.

> +               return;
> +
> +       if (IS_ERR_OR_NULL(shm->ctx) || !virt_addr_valid(shm->ctx))
> +               return;
> +
> +       if (IS_ERR_OR_NULL(shm->ctx->teedev) || !virt_addr_valid(shm->ctx->teedev))
>                 return;

shm->ctx or shm->ctx->teedev should never be an ERR pointer. The
virt_addr_valid() test doesn't make sense.

Cheers,
Jens

>
>         teedev = shm->ctx->teedev;
> --
> 2.25.1
>