stretch icon indicating copy to clipboard operation
stretch copied to clipboard

Unwind safety issues for FFI functions

Open VaynNecol opened this issue 3 years ago • 2 comments

#[no_mangle]
pub unsafe extern "C" fn stretch_node_compute_layout(
    stretch: *mut c_void,
    node: *mut c_void,
    width: f32,
    height: f32,
    create_layout: fn(*const f32) -> *mut c_void,
) -> *mut c_void {
    let mut stretch = Box::from_raw(stretch as *mut Stretch);
    let node = Box::from_raw(node as *mut Node);

    stretch
        .compute_layout(
            *node,
            Size {
                width: if f32::is_nan(width) { Number::Undefined } else { Number::Defined(width) },
                height: if f32::is_nan(height) { Number::Undefined } else { Number::Defined(height) },
            },
        )
        .unwrap();

    let mut output = vec![];
    copy_output(&stretch, *node, &mut output);

    Box::leak(stretch);
    Box::leak(node);

    create_layout(output.as_ptr())
}

The unwinding of stretch.compute_layout will introduce double free due to the deallocation of stretch and node.

#[no_mangle]
pub unsafe extern "C" fn stretch_node_compute_layout(
    stretch: *mut c_void,
    node: *mut c_void,
    width: f32,
    height: f32,
    create_layout: fn(*const f32) -> *mut c_void,
) -> *mut c_void {
    let mut stretch = mem::ManuallyDrop::new(Box::from_raw(stretch as *mut Stretch));
    let node = mem::ManuallyDrop::new(Box::from_raw(node as *mut Node));

    stretch
        .compute_layout(
            *node,
            Size {
                width: if f32::is_nan(width) { Number::Undefined } else { Number::Defined(width) },
                height: if f32::is_nan(height) { Number::Undefined } else { Number::Defined(height) },
            },
        )
        .unwrap();

    let mut output = vec![];
    copy_output(&stretch, *node, &mut output);
    create_layout(output.as_ptr())
}

The modification is zero-cost.

VaynNecol avatar May 02 '22 14:05 VaynNecol

Can you make this issue/PR against http://github.com/dioxusLabs/stretch/ ? We're maintaining an active fork with fixes and improvements.

jkelleyrtp avatar May 03 '22 02:05 jkelleyrtp

Can you make this issue/PR against http://github.com/dioxusLabs/stretch/ ? We're maintaining an active fork with fixes and improvements.

sure,please check on the issue in your fork, and we give a more detailed description

VaynNecol avatar May 04 '22 07:05 VaynNecol