apache / incubator-teaclave-trustzone-sdk

Teaclave TrustZone SDK enables safe, functional, and ergonomic development of trustlets.
https://teaclave.apache.org
Apache License 2.0
203 stars 58 forks source link

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

Open a21152 opened 5 months ago

a21152 commented 5 months ago

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 commented 5 months ago

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

a21152 commented 5 months ago

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!");
    }
    }
  3. For an Err return from fn invoke_command.
  4. Run the example CA and you shall observe drop being printed twice which is indicative of a double-free bug.
a21152 commented 2 months ago

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

https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/127