Skip to content

Add safe wrappers to automatically free mbedtls types#1974

Merged
achamayou merged 15 commits into
microsoft:masterfrom
eddyashton:mbedtls_safe_wrappers
Dec 3, 2020
Merged

Add safe wrappers to automatically free mbedtls types#1974
achamayou merged 15 commits into
microsoft:masterfrom
eddyashton:mbedtls_safe_wrappers

Conversation

@eddyashton
Copy link
Copy Markdown
Member

To try and stop the general class of issues described in #738, we need to support 2 slightly different things:

  1. Always call the relevant _free() if we've called _init(), for mbedtls structs. We could do this with a non-pointer auto-freeing wrapper struct.
  2. Free things allocated in constructors.

The latter could be done with a pattern like try {} catch { X.reset(); }, but I think this approach is cleaner. Constructors populate local temporary structs, and only retain those (move-assigning them into members) at the end of the constructor, if it was successful. The wrapper type is a unique_ptr because it gives us the custom deleter hook and move-only semantics already, but we could write our own instead to remove the additional pointer indirection.

The wrapper is generated by a macro, producing a unique_ptr with a custom deleter but also a make_unique function that will call _init, for symmetry and safety. The interesting file is mbedtls_wrappers.h, the rest is just renames (mbedtls_foo => mbedtls::Foo, mbedtls_bar(&foo...) => mbedtls_bar(foo.get()...)).

@eddyashton eddyashton requested a review from a team as a code owner December 2, 2020 15:40
@ghost
Copy link
Copy Markdown

ghost commented Dec 2, 2020

mbedtls_safe_wrappers@16233 aka 20201203.11 vs master ewma over 50 builds from 15487 to 16228
images

@achamayou achamayou merged commit 6b32335 into microsoft:master Dec 3, 2020
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

Successfully merging this pull request may close these issues.

3 participants