Comments (13)
Wentao Sun [email protected]
Thank you
from optee_os.
It seems that the thread holding tee_ta_mutex
either is killed by the OOM killer or perhaps is blocked by something else in the kernel driver.
from optee_os.
I find that the deadlock reason is session ref_count != 1.
so I think should put session before close when panicked, right?
diff --git a/core/kernel/tee_ta_manager.c b/core/kernel/tee_ta_manager.c
index b9fd3201e..9e2c84cbe 100644
--- a/core/kernel/tee_ta_manager.c
+++ b/core/kernel/tee_ta_manager.c
@@ -727,6 +727,7 @@ TEE_Result tee_ta_open_session(TEE_ErrorOrigin *err,
if (!ctx || ctx->panicked) {
DMSG("panicked, call tee_ta_close_session()");
+ tee_ta_put_session(s);
tee_ta_close_session(s, open_sessions, KERN_IDENTITY);
*err = TEE_ORIGIN_TEE;
return TEE_ERROR_TARGET_DEAD;
Thanks
from optee_os.
Tricky! Yes, it looks like a tee_ta_put_session()
is missing. How about using the common exit path of the function as:
--- a/core/kernel/tee_ta_manager.c
+++ b/core/kernel/tee_ta_manager.c
@@ -745,9 +745,9 @@ TEE_Result tee_ta_open_session(TEE_ErrorOrigin *err,
if (!ctx || ctx->panicked) {
DMSG("panicked, call tee_ta_close_session()");
- tee_ta_close_session(s, open_sessions, KERN_IDENTITY);
- *err = TEE_ORIGIN_TEE;
- return TEE_ERROR_TARGET_DEAD;
+ res = TEE_ERROR_TARGET_DEAD;
+ panicked = true;
+ goto out;
}
*sess = s;
@@ -768,6 +768,7 @@ TEE_Result tee_ta_open_session(TEE_ErrorOrigin *err,
panicked = ctx->panicked;
s->param = NULL;
+out:
/*
* Origin error equal to TEE_ORIGIN_TRUSTED_APP for "regular" error,
* apart from panicking.
Does that work?
from optee_os.
hi @jenswi-linaro ,
I used your patch, and there has been no deadlock issue by now. But there is other issue:
thread 2 destroyed ctx, but thread 1 still in enter_invoke_cmd, TEE will panic somewhere.
I try to fix it as:
diff --git a/core/kernel/tee_ta_manager.c b/core/kernel/tee_ta_manager.c
index b9fd3201e..0f5fd6879 100644
--- a/core/kernel/tee_ta_manager.c
+++ b/core/kernel/tee_ta_manager.c
@@ -793,14 +794,14 @@ TEE_Result tee_ta_invoke_command(TEE_ErrorOrigin *err,
}
ta_ctx = ts_to_ta_ctx(ts_ctx);
- if (ta_ctx->panicked) {
- DMSG("Panicked !");
- destroy_ta_ctx_from_session(sess);
+ if (!ta_ctx) {
*err = TEE_ORIGIN_TEE;
return TEE_ERROR_TARGET_DEAD;
}
tee_ta_set_busy(ta_ctx);
+ if (ta_ctx->panicked)
+ goto out;
sess->param = param;
set_invoke_timeout(sess, cancel_req_to);
@@ -809,7 +810,9 @@ TEE_Result tee_ta_invoke_command(TEE_ErrorOrigin *err,
sess->param = NULL;
tee_ta_clear_busy(ta_ctx);
+out:
if (ta_ctx->panicked) {
+ DMSG("Panicked !");
destroy_ta_ctx_from_session(sess);
*err = TEE_ORIGIN_TEE;
return TEE_ERROR_TARGET_DEAD;
And fix the panic issues I found.
diff --git a/core/kernel/pseudo_ta.c b/core/kernel/pseudo_ta.c
index 0a85eda96..9defd0623 100644
--- a/core/kernel/pseudo_ta.c
+++ b/core/kernel/pseudo_ta.c
@@ -247,7 +247,7 @@ static const struct ts_ops pseudo_ta_ops = {
bool is_pseudo_ta_ctx(struct ts_ctx *ctx)
{
- return ctx->ops == &pseudo_ta_ops;
+ return ctx && ctx->ops == &pseudo_ta_ops;
}
diff --git a/core/include/kernel/user_mode_ctx.h b/core/include/kernel/user_mode_ctx.h
index 6d65d1470..2424808f4 100644
--- a/core/include/kernel/user_mode_ctx.h
+++ b/core/include/kernel/user_mode_ctx.h
@@ -25,8 +25,10 @@ static inline struct user_mode_ctx *to_user_mode_ctx(struct ts_ctx *ctx)
return &to_user_ta_ctx(ctx)->uctx;
else if (is_sp_ctx(ctx))
return &to_sp_ctx(ctx)->uctx;
- else
+ else if (is_stmm_ctx(ctx))
return &to_stmm_ctx(ctx)->uctx;
+ else
+ panic("no such ctx");
}
System will panic at here, I think this is as expected.
to_user_mode_ctx: panic("no such ctx");
ts_to_ta_ctx: panic("bad context");
How do you think of this?
By the way, do you think the same issue will appear in the open or close processes?
Thanks
from optee_os.
It's getting hard to follow exactly what you're proposing.
Would you mind creating a PR with the changes you think are needed at the moment?
We
By the way, do you think the same issue will appear in the open or close processes?
That can't be ruled out. Do you think it's feasible to create a couple of test cases for this to put in xtest? It's not clear to me exactly how you're testing this. I can help out with this if needed.
from optee_os.
I've been able to create a test case to reproduce the deadlock and then also the panic. I'm investigating this error further.
from optee_os.
ok, thank you.
I find one of the panic reason is:
Thread 1 trigger panic, and running in user mode or other steps,
Then thread 2 destroy the ctx.
Thread 1 still running with invalid ctx and panicked.
from optee_os.
It seems the handling of panicked TAs has a few races. I have a good test case to reproduce it so I'm looking into this now.
from optee_os.
I believe I have a fix for this in #6281. Can you confirm that it fixes the issue?
from optee_os.
ok, I'll confirm, thank you.
from optee_os.
I believe I have a fix for this in #6281. Can you confirm that it fixes the issue?
Test passed for v3.18, thank you for your help.
I'll try to apply the patch to TEE v3.8 or v2.4 next.
Is there any dependencies if I apply the patch to other version?
from optee_os.
@charles1129 if you want to give a Tested-by: Your Name <[email protected]>
tag in the discussion for #6281 please feel free to do so and we will add it to the commit message. Thanks.
from optee_os.
Related Issues (20)
- Translation fault when using mobj_phys_alloc HOT 4
- While upgrading the OP-TEE version to 4.1.0-rc, a segmentation fault occurred HOT 2
- TI plat-k3 -> Usage of the OTP keys HOT 1
- Why is MMU_NUM_ASID_PAIRS set to 64? HOT 5
- Sample test case for TEE_ALG_ECDH_DERIVE_SHARED_SECRET algorithm HOT 3
- Does OP-TEE in ROCK Pi 4 supports Ethernet connectivity? HOT 5
- Typo in `huk_subkey.h` documentation. HOT 3
- OP-TEE 4.1.0 fails to boot on zynqmp(ZCU102) HOT 8
- Transition to Kconfig? HOT 12
- SMC FIQ IRQ Interrupts timeouts HOT 3
- How to link tensorflow1 static library in TA? HOT 3
- The upper limit of TA RAM or TA_DATA_SIZE
- PKCS11 TA support CKM_RSA_X_509 HOT 3
- Execution time measurement in TEE kernel mode HOT 2
- Tests required before submitting a PR HOT 3
- boot_init_memtag is called before mmu is enabled. HOT 16
- How to setup configuration without dedicate cores for OPTEE. HOT 3
- Accessing tee_common_otp.h from TA HOT 7
- OP-TEE internal AES GCM: Counter Wrap test vector failed HOT 5
- Using Shared library by TA HOT 7
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from optee_os.