[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHUa44GB8LF4iPR2tyy8HnVCwFYBXy+B2kLyf1g9GoCmsP8xsQ@mail.gmail.com>
Date: Mon, 4 Aug 2025 14:24:49 +0200
From: Jens Wiklander <jens.wiklander@...aro.org>
To: Sumit Garg <sumit.garg@...nel.org>
Cc: Pei Xiao <xiaopei01@...inos.cn>, larper@...s.com, op-tee@...ts.trustedfirmware.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tee: fix NULL pointer dereference in tee_shm_put
On Tue, Jul 29, 2025 at 11:24 AM Sumit Garg <sumit.garg@...nel.org> wrote:
>
> On Wed, Jul 23, 2025 at 10:09:07AM +0800, Pei Xiao wrote:
> > tee_shm_put have NULL pointer dereference:
> >
> > __optee_disable_shm_cache -->
> > shm = reg_pair_to_ptr(...);//shm maybe return NULL
> > tee_shm_free(shm); -->
> > tee_shm_put(shm);//crash
> >
> > Add check in tee_shm_put to fix it.
> >
> > panic log:
> > Unable to handle kernel paging request at virtual address 0000000000100cca
> > Mem abort info:
> > ESR = 0x0000000096000004
> > EC = 0x25: DABT (current EL), IL = 32 bits
> > SET = 0, FnV = 0
> > EA = 0, S1PTW = 0
> > FSC = 0x04: level 0 translation fault
> > Data abort info:
> > ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> > CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> > GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> > user pgtable: 4k pages, 48-bit VAs, pgdp=0000002049d07000
> > [0000000000100cca] pgd=0000000000000000, p4d=0000000000000000
> > Internal error: Oops: 0000000096000004 [#1] SMP
> > CPU: 2 PID: 14442 Comm: systemd-sleep Tainted: P OE ------- ----
> > 6.6.0-39-generic #38
> > Source Version: 938b255f6cb8817c95b0dd5c8c2944acfce94b07
> > Hardware name: greatwall GW-001Y1A-FTH, BIOS Great Wall BIOS V3.0
> > 10/26/2022
> > pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > pc : tee_shm_put+0x24/0x188
> > lr : tee_shm_free+0x14/0x28
> > sp : ffff001f98f9faf0
> > x29: ffff001f98f9faf0 x28: ffff0020df543cc0 x27: 0000000000000000
> > x26: ffff001f811344a0 x25: ffff8000818dac00 x24: ffff800082d8d048
> > x23: ffff001f850fcd18 x22: 0000000000000001 x21: ffff001f98f9fb88
> > x20: ffff001f83e76218 x19: ffff001f83e761e0 x18: 000000000000ffff
> > x17: 303a30303a303030 x16: 0000000000000000 x15: 0000000000000003
> > x14: 0000000000000001 x13: 0000000000000000 x12: 0101010101010101
> > x11: 0000000000000001 x10: 0000000000000001 x9 : ffff800080e08d0c
> > x8 : ffff001f98f9fb88 x7 : 0000000000000000 x6 : 0000000000000000
> > x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> > x2 : ffff001f83e761e0 x1 : 00000000ffff001f x0 : 0000000000100cca
> > Call trace:
> > tee_shm_put+0x24/0x188
> > tee_shm_free+0x14/0x28
> > __optee_disable_shm_cache+0xa8/0x108
> > optee_shutdown+0x28/0x38
> > platform_shutdown+0x28/0x40
> > device_shutdown+0x144/0x2b0
> > kernel_power_off+0x3c/0x80
> > hibernate+0x35c/0x388
> > state_store+0x64/0x80
> > kobj_attr_store+0x14/0x28
> > sysfs_kf_write+0x48/0x60
> > kernfs_fop_write_iter+0x128/0x1c0
> > vfs_write+0x270/0x370
> > ksys_write+0x6c/0x100
> > __arm64_sys_write+0x20/0x30
> > invoke_syscall+0x4c/0x120
> > el0_svc_common.constprop.0+0x44/0xf0
> > do_el0_svc+0x24/0x38
> > el0_svc+0x24/0x88
> > el0t_64_sync_handler+0x134/0x150
> > el0t_64_sync+0x14c/0x15
> >
> > Fixes: dfd0743f1d9e ("tee: handle lookup of shm with reference count 0")
> > Signed-off-by: Pei Xiao <xiaopei01@...inos.cn>
> > ---
> > drivers/tee/tee_shm.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
>
> Reviewed-by: Sumit Garg <sumit.garg@....qualcomm.com>
Looks good. I'm picking up this.
Thanks,
Jens
>
> -Sumit
>
> >
> > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > index daf6e5cfd59a..915239b033f5 100644
> > --- a/drivers/tee/tee_shm.c
> > +++ b/drivers/tee/tee_shm.c
> > @@ -560,9 +560,13 @@ EXPORT_SYMBOL_GPL(tee_shm_get_from_id);
> > */
> > void tee_shm_put(struct tee_shm *shm)
> > {
> > - struct tee_device *teedev = shm->ctx->teedev;
> > + struct tee_device *teedev;
> > bool do_release = false;
> >
> > + if (!shm || !shm->ctx || !shm->ctx->teedev)
> > + return;
> > +
> > + teedev = shm->ctx->teedev;
> > mutex_lock(&teedev->mutex);
> > if (refcount_dec_and_test(&shm->refcount)) {
> > /*
> > --
> > 2.25.1
> >
Powered by blists - more mailing lists