lkml.org 
[lkml]   [2021]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] optee: Suppress false positive kmemleak report in optee_handle_rpc()
    On Fri, 10 Dec 2021 at 09:42, Wang, Xiaolei <Xiaolei.Wang@windriver.com> wrote:
    >
    >
    >
    > -----Original Message-----
    > From: Sumit Garg <sumit.garg@linaro.org>
    > Sent: Thursday, December 9, 2021 7:41 PM
    > To: Wang, Xiaolei <Xiaolei.Wang@windriver.com>
    > Cc: jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org; linux-kernel@vger.kernel.org
    > Subject: Re: [PATCH] optee: Suppress false positive kmemleak report in optee_handle_rpc()
    >
    > [Please note: This e-mail is from an EXTERNAL e-mail address]
    >
    > On Mon, 6 Dec 2021 at 17:35, Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
    > >
    > > We observed the following kmemleak report:
    > > unreferenced object 0xffff000007904500 (size 128):
    > > comm "swapper/0", pid 1, jiffies 4294892671 (age 44.036s)
    > > hex dump (first 32 bytes):
    > > 00 47 90 07 00 00 ff ff 60 00 c0 ff 00 00 00 00 .G......`.......
    > > 60 00 80 13 00 80 ff ff a0 00 00 00 00 00 00 00 `...............
    > > backtrace:
    > > [<000000004c12b1c7>] kmem_cache_alloc+0x1ac/0x2f4
    > > [<000000005d23eb4f>] tee_shm_alloc+0x78/0x230
    > > [<00000000794dd22c>] optee_handle_rpc+0x60/0x6f0
    > > [<00000000d9f7c52d>] optee_do_call_with_arg+0x17c/0x1dc
    > > [<00000000c35884da>] optee_open_session+0x128/0x1ec
    > > [<000000001748f2ff>] tee_client_open_session+0x28/0x40
    > > [<00000000aecb5389>] optee_enumerate_devices+0x84/0x2a0
    > > [<000000003df18bf1>] optee_probe+0x674/0x6cc
    > > [<000000003a4a534a>] platform_drv_probe+0x54/0xb0
    > > [<000000000c51ce7d>] really_probe+0xe4/0x4d0
    > > [<000000002f04c865>] driver_probe_device+0x58/0xc0
    > > [<00000000b485397d>] device_driver_attach+0xc0/0xd0
    > > [<00000000c835f0df>] __driver_attach+0x84/0x124
    > > [<000000008e5a429c>] bus_for_each_dev+0x70/0xc0
    > > [<000000001735e8a8>] driver_attach+0x24/0x30
    > > [<000000006d94b04f>] bus_add_driver+0x104/0x1ec
    > >
    > > This is not a memory leak because we pass the share memory pointer to
    > > secure world and would get it from secure world before releasing it.
    >
    > > How about if it's actually a memory leak caused by the secure world?
    > > An example being secure world just allocates kernel memory via OPTEE_SMC_RPC_FUNC_ALLOC and doesn't free it via OPTEE_SMC_RPC_FUNC_FREE.
    >
    > > IMO, we need to cross-check optee-os if it's responsible for leaking kernel memory.
    >
    > Hi sumit,
    >
    > You mean we need to check whether there is a real memleak,
    > If being secure world just allocate kernel memory via OPTEE_SMC_PRC_FUNC_ALLOC and until the end, there is no free
    > It via OPTEE_SMC_PRC_FUNC_FREE, then we should judge it as a memory leak, wo need to judge whether it is caused by secure os?

    Yes. AFAICT, optee-os should allocate shared memory to communicate
    with tee-supplicant. So once the communication is done, the underlying
    shared memory should be freed. I can't think of any scenario where
    optee-os should keep hold-off shared memory indefinitely.

    -Sumit

    >
    > Thanks
    > Xiaolei
    >
    >
    > > -Sumit
    >
    > >
    > > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
    > > ---
    > > drivers/tee/optee/smc_abi.c | 2 ++
    > > 1 file changed, 2 insertions(+)
    > >
    > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
    > > index 6196d7c3888f..cf2e3293567d 100644
    > > --- a/drivers/tee/optee/smc_abi.c
    > > +++ b/drivers/tee/optee/smc_abi.c
    > > @@ -23,6 +23,7 @@
    > > #include "optee_private.h"
    > > #include "optee_smc.h"
    > > #include "optee_rpc_cmd.h"
    > > +#include <linux/kmemleak.h>
    > > #define CREATE_TRACE_POINTS
    > > #include "optee_trace.h"
    > >
    > > @@ -783,6 +784,7 @@ static void optee_handle_rpc(struct tee_context *ctx,
    > > param->a4 = 0;
    > > param->a5 = 0;
    > > }
    > > + kmemleak_not_leak(shm);
    > > break;
    > > case OPTEE_SMC_RPC_FUNC_FREE:
    > > shm = reg_pair_to_ptr(param->a1, param->a2);
    > > --
    > > 2.25.1
    > >

    \
     
     \ /
      Last update: 2021-12-10 06:01    [W:5.701 / U:1.852 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site