Skip to content
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

Missing mem::forget(b) in TA_InvokeCommand() error path? #119

Open
a21152 opened this issue Feb 1, 2024 · 3 comments
Open

Missing mem::forget(b) in TA_InvokeCommand() error path? #119

a21152 opened this issue Feb 1, 2024 · 3 comments

Comments

@a21152
Copy link
Contributor

a21152 commented Feb 1, 2024

I was playing with a TA built with teaclave SDK and got a panic when using the optional session_ctx parameter with a Vec inside of it.

It seems the mem::forget(b) call at https://github.com/apache/incubator-teaclave-trustzone-sdk/blob/master/optee-utee/macros/src/lib.rs#L393 also apply to the Err path and doing so indeed fixes the panic.

I am not sure if my logic is air tight. It would be great if someone could confirm.

@DemesneGH
Copy link
Contributor

Hi @a21152 , could you provide the code for reproducing the error? thanks!

@a21152
Copy link
Contributor Author

a21152 commented Feb 6, 2024

I don't have the code off my hand, but you will be able to reproduce the problem with the following simple steps.

  1. Take any of the example TA that make use of a session ctx parameter -- for example https://github.com/apache/incubator-teaclave-trustzone-sdk/blob/master/examples/diffie_hellman-rs/ta/src/main.rs#L32
  2. Implement the Drop trait on the session ctx object that just prints a message, for example.
impl Drop for DiffieHellman {
    fn drop(&mut self) {
        trace_println!("Dropping DiffieHellman!");
    }
}
  1. For an Err return from fn invoke_command.
  2. Run the example CA and you shall observe drop being printed twice which is indicative of a double-free bug.

@a21152
Copy link
Contributor Author

a21152 commented May 7, 2024

The following PR adds an example, test, and fix.

#127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants