-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix double-free bug in optee-utee #127
Conversation
Add error_handling-rs to exercise some of the failure paths in the optee_rust stack. This initial commit exposes a double-free bug in optee-utee. Test: run `error_handling-rs` in the qemu environment described at https://optee.readthedocs.io/en/latest/building/optee_with_rust.html and observe the following output. thread 'main' panicked at src/main.rs:42:5: assertion `left == right` failed left: TargetDead right: Generic note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace 101
optee-utee needs to forget the session context on invocation error as well. Test: run the `error-handling-rs` example in qemu and observe no panic. Example run: ``` Welcome to Buildroot, type root or test to login buildroot login: root 0 ```
When I run the example it fails:
It's also the "panic" but seems not the double-free panic, and Could you check it out? thanks! |
hmm... that is odd. 0xffff000c indicates TEE_ERROR_OUT_OF_MEMORY upon open_session(), which I don't expect to fail in the test. I rerun Also I suspect the Are you running against main branch by any chance (which may not work)? If you can't figure out it either, I would need your exact test procedure so I can try to replicate your env. here to further triage. |
@a21152 I've reproduced the bug in newer OP-TEE image (4.2.0), seems the prebuilt OP-TEE 3.20.0 (the The fix a2fb339 looks good to me. After fixing the bug would you prefer to keep the |
Sounds good. Although my run of tests/test_error_handling.sh (presumably with the same 3.20 prebuilts) also worked fine. Yes -- I'd like to keep the error_handling-rs example so we make sure this issue does not regress. It also allows us to add more tests against similar error paths. |
Merged, thanks! |
Thank you for your quick turn around! |
This PR fixes a double-free bug in optee-utee at https://github.com/apache/incubator-teaclave-trustzone-sdk/blob/master/optee-utee/macros/src/lib.rs#L393, where the
forget
call is forgotten for the error path.Each time a TA returns an error from a command handler, the bug causes
drop
to be called on the shared session object. A double-free-induced panic happens when a second error invocation happens to the same session or when the session closes after a single invocation error.This PR adds an example -- error_handling-rs that reproduces the error and also serve as a test in CI.