* [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.