rustls / rustls-ffi

Use Rustls from any language
Other
127 stars 30 forks source link

NULL safe set_boxed_mut_ptr/set_arc_mut_ptr #402

Closed cpu closed 6 months ago

cpu commented 6 months ago

Previously the set_boxed_mut_ptr() and set_arc_mut_ptr() helper fns used for assigning out parameters across the FFI boundary took *mut *mut C and *mut *const C for the destination argument dst. Using these safely required callers always verify that dst != NULL. In practice it's very easy to forget to do this and danger lurks!

We could modify these helpers to do NULL checking, but we tend to use them near the end of a function to assign a result in a success case and we would prefer NULL checking happen at the beginning of the function.

One proposed solution is to modify these setter functions to take &mut *mut C and &mut *const C. By using new helper macros to carefully construct a &mut from the input double pointer we can front-load the NULL check and the assignment in the set fns can proceed knowing there's no possibility for a NULL outer pointer.

This commit implements this strategy, updating the argument type of set_boxed_mut_ptr and set_arc_mut_ptr to take &mut (*const|*mut) C. New try_mut_from_ptr_ptr and try_ref_from_ptr_ptr macros allow converting from *mut *mut C and *mut *const C to the reference types, bailing early for NULL.

Resolves https://github.com/rustls/rustls-ffi/issues/380

cpu commented 6 months ago

I think the fn names and rustdoc could use some bike shedding but I wanted to get this up for input before I log off. 💤

cpu commented 6 months ago

@ctz Did you want to give this a review pass since you were interested in https://github.com/rustls/rustls-ffi/issues/380 ?

cpu commented 6 months ago

Thanks for taking a look.

Mental note to pull these into openssl-compat

404 might be of interest in that area as well.