Skip to content

Fix null-check against uninitialized pointer in Gd::from_sys_init_opt() #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 16, 2022
Merged

Fix null-check against uninitialized pointer in Gd::from_sys_init_opt() #20

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d9ff8b
Fix UB in Gd::from_sys_init_opt()
Bromeon Nov 14, 2022
4924b44
Simplify Gd::from_sys_init_opt() -- from_obj_sys() instead of from_sy…
Bromeon Nov 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 10 additions & 20 deletions godot-core/src/obj/gd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,33 +469,23 @@ impl<T: GodotClass> Gd<T> {
pub unsafe fn from_sys_init_opt(init_fn: impl FnOnce(sys::GDNativeTypePtr)) -> Option<Self> {
// Note: see _call_native_mb_ret_obj() in godot-cpp, which does things quite different (e.g. querying the instance binding).

// Much elegant
let mut is_null = false;
let outer_fn = |ptr| {
init_fn(ptr);

// ptr has type TypePtr = OpaqueObject* = Object** (in other words, the opaque encodes an Object*)
// we don't need to know if Object** is null, but if Object* (modified through Object**) is null.
let opaque_ptr = ptr as *const OpaqueObject;
if is_zeroed(*opaque_ptr) {
is_null = true;
}
};
// return_ptr has type GDNativeTypePtr = GDNativeObjectPtr* = OpaqueObject* = Object**
// (in other words, the type-ptr contains the _address_ of an object-ptr).
let mut object_ptr: sys::GDNativeObjectPtr = ptr::null_mut();
let return_ptr: *mut sys::GDNativeObjectPtr = ptr::addr_of_mut!(object_ptr);

init_fn(return_ptr as sys::GDNativeTypePtr);

// TODO forget, ready etc
let gd = Self::from_sys_init(outer_fn);
if is_null {
// We don't need to know if Object** is null, but if Object* is null; return_ptr has the address of a local (never null).
if object_ptr.is_null() {
None
} else {
Some(gd)
let obj = Gd::from_obj_sys(object_ptr); // equivalent to Gd::from_sys(return_ptr)
Some(obj)
}
}
}

fn is_zeroed(opaque: OpaqueObject) -> bool {
unsafe { std::mem::transmute::<OpaqueObject, u64>(opaque) == 0 }
}

/// Destructor with semantics depending on memory strategy.
///
/// * If this `Gd` smart pointer holds a reference-counted type, this will decrement the reference counter.
Expand Down