- OP-TEE - lists.trustedfirmware.org

Re: [PATCH] optee: Suppress false positive kmemleak report in optee_handle_rpc()

by Wang, Xiaolei

-----Original Message----- From: Sumit Garg<sumit.garg(a)linaro.org> Sent: Thursday, December 9, 2021 7:41 PM To: Wang, Xiaolei<Xiaolei.Wang(a)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(a)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. > > 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 memory leak, if being secure world just allocates kernel memory via OPTEE_SMC_RPC_FUNC_ALLOC and until the end, there is no free it via OPTEE_SMC_RPC_FUNC_FREE, then we should judge it as a memory leak. thanks xiaolei > > > -Sumit > >> Signed-off-by: Xiaolei Wang<xiaolei.wang(a)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 >>

3 years, 4 months

1
0
0 0

Hardening OP-TEE kernel and trustlets with sanitizers

by Igor Zhbanov

Hello! Is it possible to use any compiler-based sanitizers to harden OP-TEE kernel and/or trustlets? I know, there is ASAN support in the OP-TEE kernel. But can it be used with TAs? Or some other sanitizers like UBSan? Thank you.

3 years, 4 months

1
0
0 0

Fwd: Embedded DT (CFG_EMBED_DT) with dynamic shared memory (CFG_CORE_DYN_SHM) in optee-os

by Etienne Carriere

Hello John, > From: John Linn <linnj(a)xilinx.com> > Date: Thu, Nov 18, 2021 at 10:24 PM > Subject: Embedded DT (CFG_EMBED_DT) with dynamic shared memory (CFG_CORE_DYN_SHM) in optee-os > To: op-teeATlists.trustedfirmware.org <op-teeATlists.trustedfirmware.org> > > > It appears that dynamic shared memory does not work with an embedded DT, but I'm likely missing something. I have it working fine with an external DT. > > There is a bit of interaction in kernel/boot.c with the two configuration options and my testing is not seeing it work with 3.14 and master looks the same viewing it. > > get_external_fdt() is called which does not work with the embedded DT it appears to me. Indeed the current implementation gets the main memory size from the external non-secure DTB. This memory is mainly REE memory and can tbe used as shared memory. It looks reasonable to get the same info from the embedded DTB instead but the point to discuss, IMO, is whether the memory nodes of OP-TEE secure DT relate to OP-TEE "secure memory" or to system-wide (possibly non-secure) memory. In the former case, that information could not be used to define the "non-secure shareable address ranges". Feel free to create a P-R in optee_os for that purpose (something like try with embedded_dt() then fallback to externalè_dt()), Regards, Etienne > > Any hints or advice? > > Thanks > John

3 years, 4 months

2
2
0 0

[GIT PULL] AMD-TEE fix for v5.16

by Jens Wiklander

Hello arm-soc maintainers, Please pull this AMDTEE driver fix which takes care of a bug where IS_ERR() was used instead of a NULL check for the return value from __get_free_pages(). Note that this isn't a usual Arm driver update. This targets AMD instead, but is part of the TEE subsystem. Thanks, Jens The following changes since commit d58071a8a76d779eedab38033ae4c821c30295a5: Linux 5.16-rc3 (2021-11-28 14:09:19 -0800) are available in the Git repository at: git://git.linaro.org/people/jens.wiklander/linux-tee.git tags/amdtee-fix-for-v5.16 for you to fetch changes up to 9d7482771fac8d8e38e763263f2ca0ca12dd22c6: tee: amdtee: fix an IS_ERR() vs NULL bug (2021-11-29 09:55:49 +0100) ---------------------------------------------------------------- AMD-TEE fix IS_ERR() bug ---------------------------------------------------------------- Dan Carpenter (1): tee: amdtee: fix an IS_ERR() vs NULL bug drivers/tee/amdtee/core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

3 years, 4 months

1
0
0 0

[PATCH] tee: amdtee: fix an IS_ERR() vs NULL bug

by Dan Carpenter

The __get_free_pages() function does not return error pointers it returns NULL so fix this condition to avoid a NULL dereference. Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- drivers/tee/amdtee/core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index da6b88e80dc0..297dc62bca29 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -203,9 +203,8 @@ static int copy_ta_binary(struct tee_context *ctx, void *ptr, void **ta, *ta_size = roundup(fw->size, PAGE_SIZE); *ta = (void *)__get_free_pages(GFP_KERNEL, get_order(*ta_size)); - if (IS_ERR(*ta)) { - pr_err("%s: get_free_pages failed 0x%llx\n", __func__, - (u64)*ta); + if (!*ta) { + pr_err("%s: get_free_pages failed\n", __func__); rc = -ENOMEM; goto rel_fw; } -- 2.20.1

3 years, 4 months

3
2
0 0

[PATCH -next v2] optee: Fix NULL but dereferenced coccicheck error

by Yang Li

Eliminate the following coccicheck warning: ./drivers/tee/optee/smc_abi.c:1508:12-15: ERROR: optee is NULL but dereferenced. Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Fixes: 'commit 6749e69c4dad ("optee: add asynchronous notifications")' Signed-off-by: Yang Li <yang.lee(a)linux.alibaba.com> --- change in v2: --According to Jens's suggestion fixed as: if (memremaped_shm) memunmap(memremaped_shm); drivers/tee/optee/smc_abi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 92759d7..d7c8235 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1505,8 +1505,8 @@ static int optee_probe(struct platform_device *pdev) kfree(optee); err_free_pool: tee_shm_pool_free(pool); - if (optee->smc.memremaped_shm) - memunmap(optee->smc.memremaped_shm); + if (memremaped_shm) + memunmap(memremaped_shm); return rc; } -- 1.8.3.1

3 years, 4 months

2
1
0 0

Re: [PATCH] tee: amdtee: fix an IS_ERR() vs NULL bug

by Dan Carpenter

On Mon, Nov 29, 2021 at 04:31:51PM +0800, 994605959 wrote: > maybe try this? > - if (IS_ERR(*ta)) { > - pr_err("%s: get_free_pages failed 0x%llx\n", __func__, > - (u64)*ta); > + if (IS_ERR(ta)) { > + pr_err("%s: get_free_pages failed %p\n", __func__, ta); No, what you are suggesting is totally wrong. You are checking the wrong variable for the wrong thing. regards, dan carpenter

3 years, 4 months

1
0
0 0

Linaro OP-TEE Contributions (LOC) monthly meeting November 2021

by Ruchika Gupta

Hi, OP-TEE Contributions (LOC) monthly meeting is planned for Thursday Nov 25 @17.00 (UTC+2). Following topic is on the agenda: - Walkthrough of proposal on sharing of hardware resources between multiple secure entities - Jens Wiklander. If you have any other topics you'd like to discuss, please let us know and we can schedule them. Meeting details: --------------- Date/time: Nov 25(a)16.00 (UTC) https://everytimezone.com/s/a43c71b2 Connection details: https://www.trustedfirmware.org/meetings/ Meeting notes: http://bit.ly/loc-notes Regards, Ruchika on behalf of the Linaro OP-TEE team

3 years, 4 months

1
2
0 0

[PATCH -next] optee: Fix NULL but dereferenced coccicheck error

by Yang Li

Eliminate the following coccicheck warning: ./drivers/tee/optee/smc_abi.c:1508:12-15: ERROR: optee is NULL but dereferenced. Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Fixes: 'commit 6749e69c4dad ("optee: add asynchronous notifications")' Signed-off-by: Yang Li <yang.lee(a)linux.alibaba.com> --- drivers/tee/optee/smc_abi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index 92759d7..1f471ff 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1505,8 +1505,6 @@ static int optee_probe(struct platform_device *pdev) kfree(optee); err_free_pool: tee_shm_pool_free(pool); - if (optee->smc.memremaped_shm) - memunmap(optee->smc.memremaped_shm); return rc; } -- 1.8.3.1

3 years, 5 months

2
1
0 0

Embedded DT (CFG_EMBED_DT) with dynamic shared memory (CFG_CORE_DYN_SHM) in optee-os

by John Linn

It appears that dynamic shared memory does not work with an embedded DT, but I'm likely missing something. I have it working fine with an external DT. There is a bit of interaction in kernel/boot.c with the two configuration options and my testing is not seeing it work with 3.14 and master looks the same viewing it. get_external_fdt() is called which does not work with the embedded DT it appears to me. Any hints or advice? Thanks John #ifdef CFG_CORE_DYN_SHM static void discover_nsec_memory(void) { struct core_mmu_phys_mem *mem; const struct core_mmu_phys_mem *mem_begin = NULL; const struct core_mmu_phys_mem *mem_end = NULL; size_t nelems; void *fdt = get_external_dt(); if (fdt) { mem = get_nsec_memory(fdt, &nelems); if (mem) { core_mmu_set_discovered_nsec_ddr(mem, nelems); return; } DMSG("No non-secure memory found in FDT"); } mem_begin = phys_ddr_overall_begin; mem_end = phys_ddr_overall_end; nelems = mem_end - mem_begin; if (nelems) { /* * Platform cannot use both register_ddr() and the now * deprecated register_dynamic_shm(). */ assert(phys_ddr_overall_compat_begin == phys_ddr_overall_compat_end); } else { mem_begin = phys_ddr_overall_compat_begin; mem_end = phys_ddr_overall_compat_end; nelems = mem_end - mem_begin; if (!nelems) return; This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

3 years, 5 months

1
0
0 0

2025

2024

2023

2022

2021

2020

OP-TEE