PSA: New API to stop use-after-frees when using LLCPP client bindings

[Yifei Teng]

~ If you don't use the low-level C++ bindings you may stop reading now ~

TLDR: When making two-way client FIDL calls, instead of passing a callback you now may choose between .Then(callback) and .ThenExactlyOnce(callback) to specify the desired cancellation semantics.

Scroll to the bottom if you only have 30 seconds.

Tracking bug: 94402

 

Motivation

When making an asynchronous two-way call, the result of that call is delivered back to the application at a later time, after the execution had already left the original scope of making the call. The asynchronous dispatcher would later invoke the follow-up logic you specified when making the call, called a continuation. This means it's easy to use objects after they are destroyed, leading to memory corruptions:

// The following snippet shows an example of use-after-free // occurring in asynchronous two-way calls. void Foo(fidl::WireClient<MyProtocol>& client) {   bool call_ok;   client->SomeAsyncMethod(       // The following lambda function represents the continuation.       [&call_ok] (fidl::WireUnownedResult<SomeMethod>& result) {         // `call_ok` has already gone out of scope.         // This would lead to memory corruption.         call_ok = result.ok();       }); }

A more insidious form of this corruption occurs when the continuation captures the this pointer, and said referenced object also owns the client. Destroying the outer object (in turn, destroying the client) causes all pending two-way calls to fail. As their continuation runs to communicate the failure, the this pointer it captured is no longer valid. Here is an example:

// The following snippet shows an example of use-after-free of `this` pointer // occurring in asynchronous two-way calls. class MyObject {  public:   void DoSomething() {     client_->MyMethod([this] (fidl::WireUnownedResult<MyMethod>& result) {       // If |MyObject| is destroyed, we would attempt to store |false|       // into a destroyed field.       ok_ = result.ok();     });   }  private:   fidl::WireClient<MyProtocol>& client_;   bool ok_; };

We could fix that use-after-free by dropping all pending callbacks when a client object destructs, but that does not mesh well with some uses of the clients expecting to be always called. To cater to both use cases, we are introducing .Then(...) and .ThenExactlyOnce(...) which will be chained after specifying the request arguments, like so:

// Before client_->SomeMethod(args, [this] (fidl::WireUnownedResult<SomeMethod>& result) { ... }); // Now client_->SomeMethod(args).Then([this] (fidl::WireUnownedResult<SomeMethod>& result) { ... });

Both Then and ThenExactlyOnce register a continuation for a two-way call. However, Then is designed to mitigate corruption cases like the above. Specifically:

• Then ensures the provided continuation will be called at most once, until the client is destroyed. You should choose Then if your continuation only captures objects with the same lifetime as the client (e.g. your user object owns the client). Destroying the user object passivates any outstanding callbacks. No concerns of use-after-free.

• ThenExactlyOnce on the other hand guarantees to call the continuation exactly once. If the client object is destroyed, the continuation receives a cancellation error. You need to ensure any referenced objects are still alive by the time the continuation runs, which may be an unspecified time after the client object is destroyed. You should choose ThenExactlyOnce if your continuation must be called exactly once, such as when interfacing with fpromise completers or FIDL server completers, or during unit tests.

 

Quick decision tree

As a rule of thumb:

• If your callback looks like client->Foo([this, use Then (note that client_ is a member variable). Example in-tree.

• If your callback looks like

• client->Foo([completer = ...], or

• client->Foo([], or

• client->Foo([&] (common in unit tests),

• use ThenExactlyOnce. Example in-tree.

Please let us know on fidl...@fuchsia.dev if you have questions or suggestions. Thanks.

Cheers,

Yifei