All of lore.kernel.org
 help / color / mirror / Atom feed
* drivers/tee double-free of page when optee_shm_register fails
@ 2021-12-10 14:53 Lars Persson
  2021-12-14  7:41 ` Sumit Garg
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Persson @ 2021-12-10 14:53 UTC (permalink / raw)
  To: op-tee

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

Hi

Me and Patrik have been tracing a kernel memory corruption bug that is 
triggered when op-tee runs out of resources and returns an error from 
the OPTEE_MSG_CMD_REGISTER_SHM call. This is yet another fall-out from 
Patrik's fuzzing of the TEE subsystem.

The symptoms would look like this when page debugging is enabled:
BUG: Bad page state in process optee_example_h  pfn:46bb0
page:(ptrval) refcount:-1 mapcount:0 mapping:00000000 index:0x0 pfn:0x46bb0
flags: 0x0(zone=0)

Our reproducer runs a loop with the TEE_IOC_SHM_ALLOC until memory runs 
out at the optee-os end (dynamic SHM enabled). The error is 100% 
reproducible with such a loop.

We have traced this down to what seems to be a miss in the memory 
ownership contract during the call to OPTEE_MSG_CMD_REGISTER_SHM.

When pool_op_alloc() detects that optee_shm_register() has failed, it 
will free the allocated page at the very end of the function. 
Unfortunately that page has already been freed because OP-TEE has sent a 
OPTEE_RPC_CMD_SHM_FREE for this shm object before returning from 
OPTEE_MSG_CMD_REGISTER_SHM. This is my conclusion based on prints added 
to the code.

I cannot write a patch for this because I am at a loss of who actually 
is supposed to trigger the free of the pages in this situation. Is there 
an API spec that makes this clear ?

BR,
  Lars

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

* Re: drivers/tee double-free of page when optee_shm_register fails
  2021-12-10 14:53 Lars Persson
@ 2021-12-14  7:41 ` Sumit Garg
  0 siblings, 0 replies; 7+ messages in thread
From: Sumit Garg @ 2021-12-14  7:41 UTC (permalink / raw)
  To: op-tee

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

Hi Lars,

On Fri, 10 Dec 2021 at 20:23, Lars Persson <larper@axis.com> wrote:
>
> Hi
>
> Me and Patrik have been tracing a kernel memory corruption bug that is
> triggered when op-tee runs out of resources and returns an error from
> the OPTEE_MSG_CMD_REGISTER_SHM call. This is yet another fall-out from
> Patrik's fuzzing of the TEE subsystem.
>
> The symptoms would look like this when page debugging is enabled:
> BUG: Bad page state in process optee_example_h  pfn:46bb0
> page:(ptrval) refcount:-1 mapcount:0 mapping:00000000 index:0x0 pfn:0x46bb0
> flags: 0x0(zone=0)
>
> Our reproducer runs a loop with the TEE_IOC_SHM_ALLOC until memory runs
> out at the optee-os end (dynamic SHM enabled). The error is 100%
> reproducible with such a loop.

Can you share a simple reproducer test application (CA and TA)?

>
> We have traced this down to what seems to be a miss in the memory
> ownership contract during the call to OPTEE_MSG_CMD_REGISTER_SHM.
>
> When pool_op_alloc() detects that optee_shm_register() has failed, it
> will free the allocated page at the very end of the function.
> Unfortunately that page has already been freed because OP-TEE has sent a
> OPTEE_RPC_CMD_SHM_FREE for this shm object before returning from
> OPTEE_MSG_CMD_REGISTER_SHM. This is my conclusion based on prints added
> to the code.

I can't see any RPC free command from OP-TEE in case the
OPTEE_MSG_CMD_REGISTER_SHM fails. Have you compared kernel addresses
for SHM during OPTEE_MSG_CMD_REGISTER_SHM and OPTEE_RPC_CMD_SHM_FREE?

>
> I cannot write a patch for this because I am at a loss of who actually
> is supposed to trigger the free of the pages in this situation. Is there
> an API spec that makes this clear ?

It should only be the pool_op_alloc() API in case
OPTEE_MSG_CMD_REGISTER_SHM fails.

-Sumit

>
> BR,
>   Lars

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

* Re: drivers/tee double-free of page when optee_shm_register fails
       [not found] < <CAFA6WYNNTr0O60ggowzqpJVso5cySO=peBTBTzVm9MzZy8b4ww@mail.gmail.com>
@ 2021-12-14  8:30 ` Patrik Lantz
  2021-12-14  9:55   ` Sumit Garg
  0 siblings, 1 reply; 7+ messages in thread
From: Patrik Lantz @ 2021-12-14  8:30 UTC (permalink / raw)
  To: op-tee

[-- Attachment #1: Type: text/plain, Size: 2283 bytes --]

Hi,


I'm attaching a reproducer (only CA, there is no TA used in this case ) and crash log for reference.


Regards,

Patrik

________________________________
From: Sumit Garg <sumit.garg@linaro.org>
Sent: Tuesday, December 14, 2021 8:41:20 AM
To: Lars Persson
Cc: op-tee(a)lists.trustedfirmware.org; Patrik Lantz
Subject: Re: drivers/tee double-free of page when optee_shm_register fails

Hi Lars,

On Fri, 10 Dec 2021 at 20:23, Lars Persson <larper@axis.com> wrote:
>
> Hi
>
> Me and Patrik have been tracing a kernel memory corruption bug that is
> triggered when op-tee runs out of resources and returns an error from
> the OPTEE_MSG_CMD_REGISTER_SHM call. This is yet another fall-out from
> Patrik's fuzzing of the TEE subsystem.
>
> The symptoms would look like this when page debugging is enabled:
> BUG: Bad page state in process optee_example_h  pfn:46bb0
> page:(ptrval) refcount:-1 mapcount:0 mapping:00000000 index:0x0 pfn:0x46bb0
> flags: 0x0(zone=0)
>
> Our reproducer runs a loop with the TEE_IOC_SHM_ALLOC until memory runs
> out at the optee-os end (dynamic SHM enabled). The error is 100%
> reproducible with such a loop.

Can you share a simple reproducer test application (CA and TA)?

>
> We have traced this down to what seems to be a miss in the memory
> ownership contract during the call to OPTEE_MSG_CMD_REGISTER_SHM.
>
> When pool_op_alloc() detects that optee_shm_register() has failed, it
> will free the allocated page at the very end of the function.
> Unfortunately that page has already been freed because OP-TEE has sent a
> OPTEE_RPC_CMD_SHM_FREE for this shm object before returning from
> OPTEE_MSG_CMD_REGISTER_SHM. This is my conclusion based on prints added
> to the code.

I can't see any RPC free command from OP-TEE in case the
OPTEE_MSG_CMD_REGISTER_SHM fails. Have you compared kernel addresses
for SHM during OPTEE_MSG_CMD_REGISTER_SHM and OPTEE_RPC_CMD_SHM_FREE?

>
> I cannot write a patch for this because I am at a loss of who actually
> is supposed to trigger the free of the pages in this situation. Is there
> an API spec that makes this clear ?

It should only be the pool_op_alloc() API in case
OPTEE_MSG_CMD_REGISTER_SHM fails.

-Sumit

>
> BR,
>   Lars



[-- Attachment #2: crashlog.txt --]
[-- Type: text/plain, Size: 2347 bytes --]

==================================================================
BUG: KASAN: double-free or invalid-free in tee_shm_alloc (/mnt/build/optee-qemu/linux/drivers/tee/tee_shm.c:210) 

CPU: 0 PID: 310 Comm: optee_example_h Not tainted 5.14.0 #10
Hardware name: Generic DT based system
(unwind_backtrace) from show_stack (/mnt/build/optee-qemu/linux/arch/arm/kernel/traps.c:254) 
(show_stack) from dump_stack_lvl (/mnt/build/optee-qemu/linux/lib/dump_stack.c:106 (discriminator 1)) 
(dump_stack_lvl) from print_address_description.constprop.0 (/mnt/build/optee-qemu/linux/mm/kasan/report.c:234) 
(print_address_description.constprop.0) from kasan_report_invalid_free (/mnt/build/optee-qemu/linux/mm/kasan/report.c:359) 
(kasan_report_invalid_free) from kfree (/mnt/build/optee-qemu/linux/./include/linux/vmstat.h:523 /mnt/build/optee-qemu/linux/mm/slub.c:3248 /mnt/build/optee-qemu/linux/mm/slub.c:4264) 
(kfree) from tee_shm_alloc (/mnt/build/optee-qemu/linux/drivers/tee/tee_shm.c:210) 
(tee_shm_alloc) from tee_ioctl (/mnt/build/optee-qemu/linux/drivers/tee/tee_core.c:296 /mnt/build/optee-qemu/linux/drivers/tee/tee_core.c:862) 
(tee_ioctl) from sys_ioctl (/mnt/build/optee-qemu/linux/fs/ioctl.c:52 /mnt/build/optee-qemu/linux/fs/ioctl.c:1029 /mnt/build/optee-qemu/linux/fs/ioctl.c:1067 /mnt/build/optee-qemu/linux/fs/ioctl.c:1055) 
(sys_ioctl) from ret_fast_syscall (/mnt/build/optee-qemu/linux/arch/arm/kernel/entry-common.S:51) 
Exception stack(0x86293fa8 to 0x86293ff0)
3fa0:                   00000016 000001a0 00000003 c010a401 20000080 00000001
3fc0: 00000016 000001a0 0049151d 00000036 47ba12d0 47ba0e10 47ba12d0 6ecaec88
3fe0: 47ba0cd0 47ba0cc0 00490e69 66c74d72

The buggy address belongs to the page:
page:(ptrval) refcount:0 mapcount:0 mapping:(ptrval) index:0x0 pfn:0x4686b
aops:0x40 ino:0
flags: 0x0(zone=0)
raw: 00000000 c0888d84 b80654bc 82401200 00000000 80200020 ffffffff 00000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
8686af80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
8686b000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>8686b080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
8686b100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
8686b180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================

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

* Re: drivers/tee double-free of page when optee_shm_register fails
  2021-12-14  8:30 ` drivers/tee double-free of page when optee_shm_register fails Patrik Lantz
@ 2021-12-14  9:55   ` Sumit Garg
  0 siblings, 0 replies; 7+ messages in thread
From: Sumit Garg @ 2021-12-14  9:55 UTC (permalink / raw)
  To: op-tee

[-- Attachment #1: Type: text/plain, Size: 2675 bytes --]

On Tue, 14 Dec 2021 at 14:00, Patrik Lantz <Patrik.Lantz@axis.com> wrote:
>
> Hi,
>
>
> I'm attaching a reproducer (only CA, there is no TA used in this case ) and crash log for reference.
>

I compiled and ran this CA on my Qemuv8 setup (using the latest OP-TEE
build master) and I can't reproduce this double-free issue. Can you
share details regarding your OP-TEE setup?

-Sumit

>
> Regards,
>
> Patrik
>
> ________________________________
> From: Sumit Garg <sumit.garg@linaro.org>
> Sent: Tuesday, December 14, 2021 8:41:20 AM
> To: Lars Persson
> Cc: op-tee(a)lists.trustedfirmware.org; Patrik Lantz
> Subject: Re: drivers/tee double-free of page when optee_shm_register fails
>
> Hi Lars,
>
> On Fri, 10 Dec 2021 at 20:23, Lars Persson <larper@axis.com> wrote:
> >
> > Hi
> >
> > Me and Patrik have been tracing a kernel memory corruption bug that is
> > triggered when op-tee runs out of resources and returns an error from
> > the OPTEE_MSG_CMD_REGISTER_SHM call. This is yet another fall-out from
> > Patrik's fuzzing of the TEE subsystem.
> >
> > The symptoms would look like this when page debugging is enabled:
> > BUG: Bad page state in process optee_example_h  pfn:46bb0
> > page:(ptrval) refcount:-1 mapcount:0 mapping:00000000 index:0x0 pfn:0x46bb0
> > flags: 0x0(zone=0)
> >
> > Our reproducer runs a loop with the TEE_IOC_SHM_ALLOC until memory runs
> > out at the optee-os end (dynamic SHM enabled). The error is 100%
> > reproducible with such a loop.
>
> Can you share a simple reproducer test application (CA and TA)?
>
> >
> > We have traced this down to what seems to be a miss in the memory
> > ownership contract during the call to OPTEE_MSG_CMD_REGISTER_SHM.
> >
> > When pool_op_alloc() detects that optee_shm_register() has failed, it
> > will free the allocated page at the very end of the function.
> > Unfortunately that page has already been freed because OP-TEE has sent a
> > OPTEE_RPC_CMD_SHM_FREE for this shm object before returning from
> > OPTEE_MSG_CMD_REGISTER_SHM. This is my conclusion based on prints added
> > to the code.
>
> I can't see any RPC free command from OP-TEE in case the
> OPTEE_MSG_CMD_REGISTER_SHM fails. Have you compared kernel addresses
> for SHM during OPTEE_MSG_CMD_REGISTER_SHM and OPTEE_RPC_CMD_SHM_FREE?
>
> >
> > I cannot write a patch for this because I am at a loss of who actually
> > is supposed to trigger the free of the pages in this situation. Is there
> > an API spec that makes this clear ?
>
> It should only be the pool_op_alloc() API in case
> OPTEE_MSG_CMD_REGISTER_SHM fails.
>
> -Sumit
>
> >
> > BR,
> >   Lars

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

* Re: drivers/tee double-free of page when optee_shm_register fails
       [not found] < <CAFA6WYObPg3gTh9Vas55ae7AhRHWHV7sLNq8=BPNYKqK2aXS8g@mail.gmail.com>
@ 2021-12-14 11:25 ` Patrik Lantz
  2021-12-15 10:27   ` Sumit Garg
  0 siblings, 1 reply; 7+ messages in thread
From: Patrik Lantz @ 2021-12-14 11:25 UTC (permalink / raw)
  To: op-tee

[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]

I'm running the version from [1] (5.14.0) with CONFIG_KASAN=y and CONFIG_KASAN_INLINE=y.


[1] https://optee.readthedocs.io/en/latest/faq/faq.html#faq-try-optee
________________________________
From: Sumit Garg <sumit.garg@linaro.org>
Sent: Tuesday, December 14, 2021 10:55:53 AM
To: Patrik Lantz
Cc: Lars Persson; op-tee(a)lists.trustedfirmware.org
Subject: Re: drivers/tee double-free of page when optee_shm_register fails

On Tue, 14 Dec 2021 at 14:00, Patrik Lantz <Patrik.Lantz@axis.com> wrote:
>
> Hi,
>
>
> I'm attaching a reproducer (only CA, there is no TA used in this case ) and crash log for reference.
>

I compiled and ran this CA on my Qemuv8 setup (using the latest OP-TEE
build master) and I can't reproduce this double-free issue. Can you
share details regarding your OP-TEE setup?

-Sumit

>
> Regards,
>
> Patrik
>
> ________________________________
> From: Sumit Garg <sumit.garg@linaro.org>
> Sent: Tuesday, December 14, 2021 8:41:20 AM
> To: Lars Persson
> Cc: op-tee(a)lists.trustedfirmware.org; Patrik Lantz
> Subject: Re: drivers/tee double-free of page when optee_shm_register fails
>
> Hi Lars,
>
> On Fri, 10 Dec 2021 at 20:23, Lars Persson <larper@axis.com> wrote:
> >
> > Hi
> >
> > Me and Patrik have been tracing a kernel memory corruption bug that is
> > triggered when op-tee runs out of resources and returns an error from
> > the OPTEE_MSG_CMD_REGISTER_SHM call. This is yet another fall-out from
> > Patrik's fuzzing of the TEE subsystem.
> >
> > The symptoms would look like this when page debugging is enabled:
> > BUG: Bad page state in process optee_example_h  pfn:46bb0
> > page:(ptrval) refcount:-1 mapcount:0 mapping:00000000 index:0x0 pfn:0x46bb0
> > flags: 0x0(zone=0)
> >
> > Our reproducer runs a loop with the TEE_IOC_SHM_ALLOC until memory runs
> > out at the optee-os end (dynamic SHM enabled). The error is 100%
> > reproducible with such a loop.
>
> Can you share a simple reproducer test application (CA and TA)?
>
> >
> > We have traced this down to what seems to be a miss in the memory
> > ownership contract during the call to OPTEE_MSG_CMD_REGISTER_SHM.
> >
> > When pool_op_alloc() detects that optee_shm_register() has failed, it
> > will free the allocated page at the very end of the function.
> > Unfortunately that page has already been freed because OP-TEE has sent a
> > OPTEE_RPC_CMD_SHM_FREE for this shm object before returning from
> > OPTEE_MSG_CMD_REGISTER_SHM. This is my conclusion based on prints added
> > to the code.
>
> I can't see any RPC free command from OP-TEE in case the
> OPTEE_MSG_CMD_REGISTER_SHM fails. Have you compared kernel addresses
> for SHM during OPTEE_MSG_CMD_REGISTER_SHM and OPTEE_RPC_CMD_SHM_FREE?
>
> >
> > I cannot write a patch for this because I am at a loss of who actually
> > is supposed to trigger the free of the pages in this situation. Is there
> > an API spec that makes this clear ?
>
> It should only be the pool_op_alloc() API in case
> OPTEE_MSG_CMD_REGISTER_SHM fails.
>
> -Sumit
>
> >
> > BR,
> >   Lars

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

* Re: drivers/tee double-free of page when optee_shm_register fails
  2021-12-14 11:25 ` Patrik Lantz
@ 2021-12-15 10:27   ` Sumit Garg
  0 siblings, 0 replies; 7+ messages in thread
From: Sumit Garg @ 2021-12-15 10:27 UTC (permalink / raw)
  To: op-tee

[-- Attachment #1: Type: text/plain, Size: 4321 bytes --]

On Tue, 14 Dec 2021 at 16:55, Patrik Lantz <Patrik.Lantz@axis.com> wrote:
>
> I'm running the version from [1] (5.14.0) with CONFIG_KASAN=y and CONFIG_KASAN_INLINE=y.
>

Now I am able to reproduce, found the root cause and fixed it here
[1]. For your Linux version, the fix would be as follows. Give it a
try and let me know if it works for you as well.

diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c
index c41a9a501..fa75024f1 100644
--- a/drivers/tee/optee/shm_pool.c
+++ b/drivers/tee/optee/shm_pool.c
@@ -41,10 +41,8 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
                        goto err;
                }

-               for (i = 0; i < nr_pages; i++) {
-                       pages[i] = page;
-                       page++;
-               }
+               for (i = 0; i < nr_pages; i++)
+                       pages[i] = page + i;

                shm->flags |= TEE_SHM_REGISTER;
                rc = optee_shm_register(shm->ctx, shm, pages, nr_pages,

[1] https://lists.trustedfirmware.org/pipermail/op-tee/2021-December/000995.html

-Sumit

>
> [1] https://optee.readthedocs.io/en/latest/faq/faq.html#faq-try-optee
> ________________________________
> From: Sumit Garg <sumit.garg@linaro.org>
> Sent: Tuesday, December 14, 2021 10:55:53 AM
> To: Patrik Lantz
> Cc: Lars Persson; op-tee(a)lists.trustedfirmware.org
> Subject: Re: drivers/tee double-free of page when optee_shm_register fails
>
> On Tue, 14 Dec 2021 at 14:00, Patrik Lantz <Patrik.Lantz@axis.com> wrote:
> >
> > Hi,
> >
> >
> > I'm attaching a reproducer (only CA, there is no TA used in this case ) and crash log for reference.
> >
>
> I compiled and ran this CA on my Qemuv8 setup (using the latest OP-TEE
> build master) and I can't reproduce this double-free issue. Can you
> share details regarding your OP-TEE setup?
>
> -Sumit
>
> >
> > Regards,
> >
> > Patrik
> >
> > ________________________________
> > From: Sumit Garg <sumit.garg@linaro.org>
> > Sent: Tuesday, December 14, 2021 8:41:20 AM
> > To: Lars Persson
> > Cc: op-tee(a)lists.trustedfirmware.org; Patrik Lantz
> > Subject: Re: drivers/tee double-free of page when optee_shm_register fails
> >
> > Hi Lars,
> >
> > On Fri, 10 Dec 2021 at 20:23, Lars Persson <larper@axis.com> wrote:
> > >
> > > Hi
> > >
> > > Me and Patrik have been tracing a kernel memory corruption bug that is
> > > triggered when op-tee runs out of resources and returns an error from
> > > the OPTEE_MSG_CMD_REGISTER_SHM call. This is yet another fall-out from
> > > Patrik's fuzzing of the TEE subsystem.
> > >
> > > The symptoms would look like this when page debugging is enabled:
> > > BUG: Bad page state in process optee_example_h  pfn:46bb0
> > > page:(ptrval) refcount:-1 mapcount:0 mapping:00000000 index:0x0 pfn:0x46bb0
> > > flags: 0x0(zone=0)
> > >
> > > Our reproducer runs a loop with the TEE_IOC_SHM_ALLOC until memory runs
> > > out at the optee-os end (dynamic SHM enabled). The error is 100%
> > > reproducible with such a loop.
> >
> > Can you share a simple reproducer test application (CA and TA)?
> >
> > >
> > > We have traced this down to what seems to be a miss in the memory
> > > ownership contract during the call to OPTEE_MSG_CMD_REGISTER_SHM.
> > >
> > > When pool_op_alloc() detects that optee_shm_register() has failed, it
> > > will free the allocated page at the very end of the function.
> > > Unfortunately that page has already been freed because OP-TEE has sent a
> > > OPTEE_RPC_CMD_SHM_FREE for this shm object before returning from
> > > OPTEE_MSG_CMD_REGISTER_SHM. This is my conclusion based on prints added
> > > to the code.
> >
> > I can't see any RPC free command from OP-TEE in case the
> > OPTEE_MSG_CMD_REGISTER_SHM fails. Have you compared kernel addresses
> > for SHM during OPTEE_MSG_CMD_REGISTER_SHM and OPTEE_RPC_CMD_SHM_FREE?
> >
> > >
> > > I cannot write a patch for this because I am at a loss of who actually
> > > is supposed to trigger the free of the pages in this situation. Is there
> > > an API spec that makes this clear ?
> >
> > It should only be the pool_op_alloc() API in case
> > OPTEE_MSG_CMD_REGISTER_SHM fails.
> >
> > -Sumit
> >
> > >
> > > BR,
> > >   Lars

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

* Re: drivers/tee double-free of page when optee_shm_register fails
       [not found] < <CAFA6WYPFjiOhTnyi-NFCMhiXdsQr7Ro_YzXY-20Gg4e57Y8tEA@mail.gmail.com>
@ 2021-12-16  8:09 ` Patrik Lantz
  0 siblings, 0 replies; 7+ messages in thread
From: Patrik Lantz @ 2021-12-16  8:09 UTC (permalink / raw)
  To: op-tee

[-- Attachment #1: Type: text/plain, Size: 4681 bytes --]

Sorry for the delayed response.


This fix works for me.


Thanks,

Patrik

________________________________
From: Sumit Garg <sumit.garg@linaro.org>
Sent: Wednesday, December 15, 2021 11:27:47 AM
To: Patrik Lantz
Cc: Lars Persson; op-tee(a)lists.trustedfirmware.org
Subject: Re: drivers/tee double-free of page when optee_shm_register fails

On Tue, 14 Dec 2021 at 16:55, Patrik Lantz <Patrik.Lantz@axis.com> wrote:
>
> I'm running the version from [1] (5.14.0) with CONFIG_KASAN=y and CONFIG_KASAN_INLINE=y.
>

Now I am able to reproduce, found the root cause and fixed it here
[1]. For your Linux version, the fix would be as follows. Give it a
try and let me know if it works for you as well.

diff --git a/drivers/tee/optee/shm_pool.c b/drivers/tee/optee/shm_pool.c
index c41a9a501..fa75024f1 100644
--- a/drivers/tee/optee/shm_pool.c
+++ b/drivers/tee/optee/shm_pool.c
@@ -41,10 +41,8 @@ static int pool_op_alloc(struct tee_shm_pool_mgr *poolm,
                        goto err;
                }

-               for (i = 0; i < nr_pages; i++) {
-                       pages[i] = page;
-                       page++;
-               }
+               for (i = 0; i < nr_pages; i++)
+                       pages[i] = page + i;

                shm->flags |= TEE_SHM_REGISTER;
                rc = optee_shm_register(shm->ctx, shm, pages, nr_pages,

[1] https://lists.trustedfirmware.org/pipermail/op-tee/2021-December/000995.html

-Sumit

>
> [1] https://optee.readthedocs.io/en/latest/faq/faq.html#faq-try-optee
> ________________________________
> From: Sumit Garg <sumit.garg@linaro.org>
> Sent: Tuesday, December 14, 2021 10:55:53 AM
> To: Patrik Lantz
> Cc: Lars Persson; op-tee(a)lists.trustedfirmware.org
> Subject: Re: drivers/tee double-free of page when optee_shm_register fails
>
> On Tue, 14 Dec 2021 at 14:00, Patrik Lantz <Patrik.Lantz@axis.com> wrote:
> >
> > Hi,
> >
> >
> > I'm attaching a reproducer (only CA, there is no TA used in this case ) and crash log for reference.
> >
>
> I compiled and ran this CA on my Qemuv8 setup (using the latest OP-TEE
> build master) and I can't reproduce this double-free issue. Can you
> share details regarding your OP-TEE setup?
>
> -Sumit
>
> >
> > Regards,
> >
> > Patrik
> >
> > ________________________________
> > From: Sumit Garg <sumit.garg@linaro.org>
> > Sent: Tuesday, December 14, 2021 8:41:20 AM
> > To: Lars Persson
> > Cc: op-tee(a)lists.trustedfirmware.org; Patrik Lantz
> > Subject: Re: drivers/tee double-free of page when optee_shm_register fails
> >
> > Hi Lars,
> >
> > On Fri, 10 Dec 2021 at 20:23, Lars Persson <larper@axis.com> wrote:
> > >
> > > Hi
> > >
> > > Me and Patrik have been tracing a kernel memory corruption bug that is
> > > triggered when op-tee runs out of resources and returns an error from
> > > the OPTEE_MSG_CMD_REGISTER_SHM call. This is yet another fall-out from
> > > Patrik's fuzzing of the TEE subsystem.
> > >
> > > The symptoms would look like this when page debugging is enabled:
> > > BUG: Bad page state in process optee_example_h  pfn:46bb0
> > > page:(ptrval) refcount:-1 mapcount:0 mapping:00000000 index:0x0 pfn:0x46bb0
> > > flags: 0x0(zone=0)
> > >
> > > Our reproducer runs a loop with the TEE_IOC_SHM_ALLOC until memory runs
> > > out at the optee-os end (dynamic SHM enabled). The error is 100%
> > > reproducible with such a loop.
> >
> > Can you share a simple reproducer test application (CA and TA)?
> >
> > >
> > > We have traced this down to what seems to be a miss in the memory
> > > ownership contract during the call to OPTEE_MSG_CMD_REGISTER_SHM.
> > >
> > > When pool_op_alloc() detects that optee_shm_register() has failed, it
> > > will free the allocated page at the very end of the function.
> > > Unfortunately that page has already been freed because OP-TEE has sent a
> > > OPTEE_RPC_CMD_SHM_FREE for this shm object before returning from
> > > OPTEE_MSG_CMD_REGISTER_SHM. This is my conclusion based on prints added
> > > to the code.
> >
> > I can't see any RPC free command from OP-TEE in case the
> > OPTEE_MSG_CMD_REGISTER_SHM fails. Have you compared kernel addresses
> > for SHM during OPTEE_MSG_CMD_REGISTER_SHM and OPTEE_RPC_CMD_SHM_FREE?
> >
> > >
> > > I cannot write a patch for this because I am at a loss of who actually
> > > is supposed to trigger the free of the pages in this situation. Is there
> > > an API spec that makes this clear ?
> >
> > It should only be the pool_op_alloc() API in case
> > OPTEE_MSG_CMD_REGISTER_SHM fails.
> >
> > -Sumit
> >
> > >
> > > BR,
> > >   Lars

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

end of thread, other threads:[~2021-12-16  8:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] < <CAFA6WYNNTr0O60ggowzqpJVso5cySO=peBTBTzVm9MzZy8b4ww@mail.gmail.com>
2021-12-14  8:30 ` drivers/tee double-free of page when optee_shm_register fails Patrik Lantz
2021-12-14  9:55   ` Sumit Garg
     [not found] < <CAFA6WYPFjiOhTnyi-NFCMhiXdsQr7Ro_YzXY-20Gg4e57Y8tEA@mail.gmail.com>
2021-12-16  8:09 ` Patrik Lantz
     [not found] < <CAFA6WYObPg3gTh9Vas55ae7AhRHWHV7sLNq8=BPNYKqK2aXS8g@mail.gmail.com>
2021-12-14 11:25 ` Patrik Lantz
2021-12-15 10:27   ` Sumit Garg
2021-12-10 14:53 Lars Persson
2021-12-14  7:41 ` Sumit Garg

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.