* [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
* Re: [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
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
0 siblings, 1 reply; 6+ messages in thread
From: Jens Wiklander @ 2025-09-16 7:46 UTC (permalink / raw)
To: Masami Ichikawa; +Cc: op-tee
Hi Masami,
Thanks for the report. Does the problem also occur with earlier
versions of the kernel? I have more comments inline below.
On Sat, Sep 13, 2025 at 8:07 AM Masami Ichikawa <masami256@gmail.com> wrote:
>
> 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
So you have a small allocation and then report a larger one in
tmpref.size. This is an out-of-bounds reference. Memory might follow
the buffer, or it might be unmapped, depending on the state of the
heap. However, this alone isn't enough for param_from_user() to return
-EINVAL as described below.
I've tried making changes in teec_pre_process_tmpref() to so it can
trigger the condition in param_from_user() below. I think I succeeded
in that, but I'm unable to reproduce the crash.
Can you double-check to see if something is missing to reproduce the problem?
>
> 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;
> }
The tee_shm_put() is supposed to balance with the earlier
tee_shm_get_from_id(), so this in itself should be OK.
>
> 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.
This doesn't make sense shm->size == 0xd690 should require 14 or 15
pages, depending on shm->offset. The tee_shm is in an inconsistent
state, besides the fact that we have NULL pointers in shm->pages[].
>
> I have confirmed that I can prevent NULL pointer dereference by making
> the following changes.
The patch below works around the problem rather than fixing it. The
problem occurred earlier when the tee_shm became inconsistent.
Cheers,
Jens
>
> 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 [flat|nested] 6+ messages in thread
* Re: [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
2025-09-16 7:46 ` Jens Wiklander
@ 2025-09-17 13:58 ` Masami Ichikawa
2025-09-18 12:25 ` Jens Wiklander
0 siblings, 1 reply; 6+ messages in thread
From: Masami Ichikawa @ 2025-09-17 13:58 UTC (permalink / raw)
To: Jens Wiklander; +Cc: op-tee
Hi Jens,
Thank you for your time to check my report.
On Tue, Sep 16, 2025 at 4:46 PM Jens Wiklander
<jens.wiklander@linaro.org> wrote:
>
> Hi Masami,
>
> Thanks for the report. Does the problem also occur with earlier
> versions of the kernel? I have more comments inline below.
>
> On Sat, Sep 13, 2025 at 8:07 AM Masami Ichikawa <masami256@gmail.com> wrote:
> >
> > 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
>
> So you have a small allocation and then report a larger one in
> tmpref.size. This is an out-of-bounds reference. Memory might follow
> the buffer, or it might be unmapped, depending on the state of the
> heap. However, this alone isn't enough for param_from_user() to return
> -EINVAL as described below.
>
> I've tried making changes in teec_pre_process_tmpref() to so it can
> trigger the condition in param_from_user() below. I think I succeeded
> in that, but I'm unable to reproduce the crash.
>
> Can you double-check to see if something is missing to reproduce the problem?
>
I wrote a test program and ran it on both 6.17-rc5 and 6.14. I was
able to reproduce the crash on both kernels.
I uploaded test code and test results to my gist.
https://gist.github.com/masami256/11e21a7503812af7ee1e890080093a2c
The test code is crash_test.c. This program takes 2 arguments. First
argument is malicious buffer size and second one is actual buffer
size.
I can reproduce the crash with the following pair.
malicious buffer size: 0xffffff
actual buffer size: 0xff
The test_log.md file contains the test results for 6.17-rc5.
> >
> > 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;
> > }
>
> The tee_shm_put() is supposed to balance with the earlier
> tee_shm_get_from_id(), so this in itself should be OK.
>
> >
> > 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.
>
> This doesn't make sense shm->size == 0xd690 should require 14 or 15
> pages, depending on shm->offset. The tee_shm is in an inconsistent
> state, besides the fact that we have NULL pointers in shm->pages[].
>
> >
> > I have confirmed that I can prevent NULL pointer dereference by making
> > the following changes.
>
> The patch below works around the problem rather than fixing it. The
> problem occurred earlier when the tee_shm became inconsistent.
>
Yes, that's right.
Please let me know if I can help with the debugging.
> Cheers,
> Jens
>
> >
> > 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
Regards,
--
Masami Ichikawa
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
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
0 siblings, 2 replies; 6+ messages in thread
From: Jens Wiklander @ 2025-09-18 12:25 UTC (permalink / raw)
To: Masami Ichikawa; +Cc: op-tee, sumit.garg
Hi Masami,
[+Sumit in CC]
On Wed, Sep 17, 2025 at 10:58:11PM +0900, Masami Ichikawa wrote:
[snip]
> I wrote a test program and ran it on both 6.17-rc5 and 6.14. I was
> able to reproduce the crash on both kernels.
>
> I uploaded test code and test results to my gist.
> https://gist.github.com/masami256/11e21a7503812af7ee1e890080093a2c
>
> The test code is crash_test.c. This program takes 2 arguments. First
> argument is malicious buffer size and second one is actual buffer
> size.
> I can reproduce the crash with the following pair.
>
> malicious buffer size: 0xffffff
> actual buffer size: 0xff
Thanks, that easily reproduces the problem. The following diff should fix it:
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -318,7 +318,16 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
len = iov_iter_extract_pages(iter, &shm->pages, LONG_MAX, num_pages, 0,
&off);
- if (unlikely(len <= 0)) {
+ if (unlikely(len < num_pages * PAGE_SIZE)) {
+ if (len > 0) {
+ /*
+ * If we only got a few pages, update to release
+ * the correct amount below.
+ */
+ shm->num_pages = len / PAGE_SIZE;
+ ret = ERR_PTR(-ENOMEM);
+ goto err_put_shm_pages;
+ }
ret = len ? ERR_PTR(len) : ERR_PTR(-ENOMEM);
goto err_free_shm_pages;
}
Cheers,
Jens
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
2025-09-18 12:25 ` Jens Wiklander
@ 2025-09-18 14:31 ` Masami Ichikawa
2025-09-19 6:05 ` Sumit Garg via OP-TEE
1 sibling, 0 replies; 6+ messages in thread
From: Masami Ichikawa @ 2025-09-18 14:31 UTC (permalink / raw)
To: Jens Wiklander; +Cc: op-tee, sumit.garg
Hi Jens,
On Thu, Sep 18, 2025 at 9:25 PM Jens Wiklander
<jens.wiklander@linaro.org> wrote:
>
> Hi Masami,
>
> [+Sumit in CC]
>
> On Wed, Sep 17, 2025 at 10:58:11PM +0900, Masami Ichikawa wrote:
> [snip]
> > I wrote a test program and ran it on both 6.17-rc5 and 6.14. I was
> > able to reproduce the crash on both kernels.
> >
> > I uploaded test code and test results to my gist.
> > https://gist.github.com/masami256/11e21a7503812af7ee1e890080093a2c
> >
> > The test code is crash_test.c. This program takes 2 arguments. First
> > argument is malicious buffer size and second one is actual buffer
> > size.
> > I can reproduce the crash with the following pair.
> >
> > malicious buffer size: 0xffffff
> > actual buffer size: 0xff
>
> Thanks, that easily reproduces the problem. The following diff should fix it:
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -318,7 +318,16 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>
> len = iov_iter_extract_pages(iter, &shm->pages, LONG_MAX, num_pages, 0,
> &off);
> - if (unlikely(len <= 0)) {
> + if (unlikely(len < num_pages * PAGE_SIZE)) {
> + if (len > 0) {
> + /*
> + * If we only got a few pages, update to release
> + * the correct amount below.
> + */
> + shm->num_pages = len / PAGE_SIZE;
> + ret = ERR_PTR(-ENOMEM);
> + goto err_put_shm_pages;
> + }
> ret = len ? ERR_PTR(len) : ERR_PTR(-ENOMEM);
> goto err_free_shm_pages;
> }
>
> Cheers,
> Jens
Thank you for the fix.
I tested both 6.17-rc5 and 6.14. I confirmed your patch solves the problem.
Tested-by: Masami Ichikawa <masami256@gmail.com>
Regards,
--
Masami Ichikawa
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
2025-09-18 12:25 ` Jens Wiklander
2025-09-18 14:31 ` Masami Ichikawa
@ 2025-09-19 6:05 ` Sumit Garg via OP-TEE
1 sibling, 0 replies; 6+ messages in thread
From: Sumit Garg via OP-TEE @ 2025-09-19 6:05 UTC (permalink / raw)
To: Jens Wiklander, Masami Ichikawa; +Cc: op-tee
On Thu, Sep 18, 2025 at 02:25:41PM +0200, Jens Wiklander wrote:
> Hi Masami,
>
> [+Sumit in CC]
>
> On Wed, Sep 17, 2025 at 10:58:11PM +0900, Masami Ichikawa wrote:
> [snip]
> > I wrote a test program and ran it on both 6.17-rc5 and 6.14. I was
> > able to reproduce the crash on both kernels.
> >
> > I uploaded test code and test results to my gist.
> > https://gist.github.com/masami256/11e21a7503812af7ee1e890080093a2c
> >
> > The test code is crash_test.c. This program takes 2 arguments. First
> > argument is malicious buffer size and second one is actual buffer
> > size.
> > I can reproduce the crash with the following pair.
> >
> > malicious buffer size: 0xffffff
> > actual buffer size: 0xff
>
Thanks Masami for the report and the bug reproducer here.
> Thanks, that easily reproduces the problem. The following diff should fix it:
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -318,7 +318,16 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>
> len = iov_iter_extract_pages(iter, &shm->pages, LONG_MAX, num_pages, 0,
> &off);
> - if (unlikely(len <= 0)) {
> + if (unlikely(len < num_pages * PAGE_SIZE)) {
> + if (len > 0) {
> + /*
> + * If we only got a few pages, update to release
> + * the correct amount below.
> + */
> + shm->num_pages = len / PAGE_SIZE;
> + ret = ERR_PTR(-ENOMEM);
> + goto err_put_shm_pages;
> + }
> ret = len ? ERR_PTR(len) : ERR_PTR(-ENOMEM);
> goto err_free_shm_pages;
> }
Thanks Jens for the fix, it sounds appropriate to me. I think this
commit [1] introduced the bug in the first place as earlier check for
pin_user_pages_fast() would have caught this issue without crashing the
kernel.
Jens, can you please send a proper fix here? I hope we should be able to
get it merged for v6.17 since it sounds critical to me.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7bdee41575919773818e525ea19e54eb817770af
-Sumit
^ permalink raw reply [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.