All of lore.kernel.org
 help / color / mirror / Atom feed
* [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
@ 2025-09-13  6:06 Masami Ichikawa
  2025-09-16  7:46 ` Jens Wiklander
  0 siblings, 1 reply; 6+ messages in thread
From: Masami Ichikawa @ 2025-09-13  6:06 UTC (permalink / raw)
  To: op-tee

Hi.

While doing my fuzzing work, I found the following kernel crash by
NULL pointer dereference in Linux 6.17-rc5.

[   16.143987] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000008
[   16.144141] Mem abort info:
[   16.144215]   ESR = 0x0000000096000004
[   16.144246]   EC = 0x25
** replaying previous printk message **
[   16.144246]   EC = 0x25: DABT (current EL), IL = 32 bits
[   16.144271]   SET = 0, FnV = 0
[   16.144289]   EA = 0, S1PTW = 0
[   16.144308]   FSC = 0x04: level 0 translation fault
[   16.144325] Data abort info:
[   16.144335]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[   16.144346]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   16.144358]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   16.144412] user pgtable: 4k pages, 52-bit VAs, pgdp=0000000048b34b00
[   16.144432] [0000000000000008] pgd=0800000040bb3403, p4d=0000000000000000
[   16.144876] Internal error: Oops: 0000000096000004 [#1]  SMP
[   16.146429] Modules linked in:
[   16.146775] CPU: 0 UID: 0 PID: 148 Comm: xtest Not tainted
6.17.0-rc5 #58 PREEMPT
[   16.146995] Hardware name: linux,dummy-virt (DT)
[   16.147181] pstate: 21402005 (nzCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[   16.147330] pc : unpin_user_pages+0x78/0xd0
[   16.147763] lr : unpin_user_pages+0xa0/0xd0
[   16.147842] sp : ffff800084403d20
[   16.147912] x29: ffff800084403d20 x28: fff00000054aa300 x27: 0000000000000000
[   16.148089] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
[   16.148235] x23: fff00000004fb5a8 x22: 0000000000000001 x21: 000000000000000d
[   16.148401] x20: fff0000000b2f9c0 x19: 0000000000000011 x18: 0000000000000001
[   16.148544] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[   16.148659] x14: 0000000000000002 x13: 0000000000000002 x12: 0000000000037d0f
[   16.148786] x11: fff0000001dad708 x10: 000000000000003f x9 : 0000000000000d1b
[   16.148925] x8 : 00000000000007e0 x7 : 0000000000000001 x6 : 000000000000000d
[   16.149039] x5 : ffffffffffffffff x4 : ffffffffffffffff x3 : 000000000000000e
[   16.149167] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffc1ffc0fd68c0
[   16.149351] Call trace:
[   16.149520]  unpin_user_pages+0x78/0xd0 (P)
[   16.149684]  tee_shm_put+0x134/0x184
[   16.149783]  tee_shm_fop_release+0x14/0x24
[   16.149866]  __fput+0xcc/0x2dc
[   16.149925]  fput_close_sync+0x40/0x108
[   16.149991]  __arm64_sys_close+0x38/0x7c
[   16.150058]  invoke_syscall+0x48/0x110
[   16.150127]  el0_svc_common.constprop.0+0x40/0xe8
[   16.150227]  do_el0_svc+0x20/0x2c
[   16.150303]  el0_svc+0x34/0xf0
[   16.150369]  el0t_64_sync_handler+0xa0/0xe4
[   16.150439]  el0t_64_sync+0x198/0x19c
[   16.150629] Code: aa0203e3 eb02027f 54000109 f8627a82 (f9400444)
[   16.150940] ---[ end trace 0000000000000000 ]---
[   16.151230] Kernel panic - not syncing: Oops: Fatal exception
[   16.151466] SMP: stopping secondary CPUs
[   16.151838] Kernel Offset: disabled
[   16.151911] CPU features: 0x000000,0000d180,2bbe33e1,957e7f3f
[   16.152019] Memory Limit: none
[   16.152284] ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---

decode_stacktrace.sh shows the following call sequence.

[   20.554057] unpin_user_pages (./include/linux/page-flags.h:284
mm/gup.c:259 mm/gup.c:420 mm/gup.c:400) (P)
[   20.554204] tee_shm_put (drivers/tee/tee_shm.c:42
drivers/tee/tee_shm.c:57 drivers/tee/tee_shm.c:587)
[   20.554291] tee_shm_fop_release (drivers/tee/tee_shm.c:437)
[   20.554366] __fput (fs/file_table.c:469)
[   20.554429] fput_close_sync (fs/file_table.c:574)
[   20.554496] __arm64_sys_close (fs/open.c:1590 fs/open.c:1572 fs/open.c:1572)
[   20.554565] invoke_syscall (./arch/arm64/include/asm/current.h:19
arch/arm64/kernel/syscall.c:54)
[   20.554639] el0_svc_common.constprop.0
(./include/linux/thread_info.h:135 arch/arm64/kernel/syscall.c:140)
[   20.554719] do_el0_svc (arch/arm64/kernel/syscall.c:152)
[   20.554782] el0_svc (./arch/arm64/include/asm/irqflags.h:55
./arch/arm64/include/asm/irqflags.h:76
arch/arm64/kernel/entry-common.c:169
arch/arm64/kernel/entry-common.c:182
arch/arm64/kernel/entry-common.c:880)
[   20.554842] el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:899)
[   20.554914] el0t_64_sync (arch/arm64/kernel/entry.S:596)

I set up a test environment using  qemu_v8.xml in the OP-TEE/manifest
repository.

## Test case

This test is based on xtest_tee_test_1004 included in xtest in the
optee_test repository. For fuzzing, the following data is created
using xtest_crypto_test().
It creates parameters like this.

op.params[0].tmpref.buffer = crypt_in;   // crypt_in =
malloc(sizeof(uint8_t) * 0xff);
op.params[0].tmpref.size = input_len;    // 0xffff
op.params[1].tmpref.buffer = crypt_out;  // crypt_out =
malloc(sizeof(uint8_t) * 0xff);
op.params[1].tmpref.size = input_len;    // 0xffff

When TEEC_InvokeCommand() in libteec was called with above parameters,
it printed folloing errorr then, linux kernel was crashed.

ERR [152] LT:TEEC_InvokeCommand:730: TEE_IOC_INVOKE failed

## Crash scenario

1. modified version of xtest_crypto_test() creates parameters.
2. xtest_crypto_test() calls TEEC_InvokeCommand() in the libteec.
3. TEEC_InvokeCommand calls ioctl(2).
4. tee_ioctl_invoke() in the tee_core.c is called by ioctl(2).
5. tee_ioctl_invoke() calles params_from_user().
6. In the params_from_user(), it returned -EINVAL because of following
if condition was true.
        if ((ip.a + ip.b) < ip.a ||
            (ip.a + ip.b) > shm->size) {
                tee_shm_put(shm);
                return -EINVAL;
        }

7. tee_ioctl_invoke() recive an error from params_from_user() so that
it run clean up process(in the out label).
8. tee_ioctl_invoke() returns -EINVAL.
9. TEEC_InvokeCommand() prints an error log then calls
teec_free_temp_refs() in libteec.
10. teec_free_temp_refs() calls TEEC_ReleaseSharedMemory().
11. TEEC_ReleaseSharedMemory() calls close(2).
12. tee_shm_put() in the tee_shm.c is called.
13. tee_shm_put() calls tee_shm_release().
14. tee_shm_release() calls release_registered_pages().
15. release_registered_pages() calls unpin_user_pages().
16. Null pointer dereference is happened in unpin_user_pages().

## Debugging

I added following debug logs in the tee_shm_release()

        } else if (shm->flags & TEE_SHM_DYNAMIC) {
                int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm);
                size_t i;
                if (rc)
                        dev_err(teedev->dev.parent,
                                "unregister shm %p failed: %d", shm, rc);

                pr_info("%s:%d: shm->num_pages: 0x%lx, shm->size:
0x%lx\n", __func__, __LINE__, shm->num_pages, shm->size);
                for (i = 0; i < shm->num_pages; i++) {
                        if (!shm->pages[i]) {
                                pr_info("%s:%d: shm->pages[%ld] is
NULL", __func__, __LINE__, i);
                        }
                }
                release_registered_pages(shm);
        }

It showed the following logs.
[   21.350894] tee_shm_release:57: shm->num_pages: 0x11, shm->size: 0xd690
[   21.350977] tee_shm_release:60: shm->pages[14] is NULL
[   21.351003] tee_shm_release:60: shm->pages[15] is NULL
[   21.351012] tee_shm_release:60: shm->pages[16] is NULL

According to the above logs, shm->num_pages is 17 but the pages array
contains NULL pointers so that it causes a NULL pointer dereference
bug.

I have confirmed that I can prevent NULL pointer dereference by making
the following changes.

diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 2a7d253d9c55..c5d39a0efbdb 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -34,9 +34,16 @@ static void shm_get_kernel_pages(struct page
**pages, size_t page_count)
 static void release_registered_pages(struct tee_shm *shm)
 {
        if (shm->pages) {
-               if (shm->flags & TEE_SHM_USER_MAPPED)
-                       unpin_user_pages(shm->pages, shm->num_pages);
-               else
+               if (shm->flags & TEE_SHM_USER_MAPPED) {
+                       size_t num_pages = 0;
+                       size_t i;
+                       for (i = 0; i < shm->num_pages; i++, num_pages++) {
+                               if (!shm->pages[i])
+                                       break;
+                       }
+
+                       unpin_user_pages(shm->pages, num_pages);
+               } else
                        shm_put_kernel_pages(shm->pages, shm->num_pages);

                kfree(shm->pages);


Regards,

-- 
Masami Ichikawa

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

end of thread, other threads:[~2025-09-19  6:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-13  6:06 [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages Masami Ichikawa
2025-09-16  7:46 ` Jens Wiklander
2025-09-17 13:58   ` Masami Ichikawa
2025-09-18 12:25     ` Jens Wiklander
2025-09-18 14:31       ` Masami Ichikawa
2025-09-19  6:05       ` Sumit Garg via OP-TEE

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.