Repository navigation
fix(other): add safety checks to from_raw_pointer #478
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # Change: Add safety checks to `from_raw_pointer` in Rust bindings | ||
|
|
||
| - **Status**: Accepted | ||
| - **Date**: 2026-04-14 | ||
| - **Tier**: Light | ||
|
|
||
| ## Overview | ||
|
|
||
| Add explicit null-pointer and alignment checks to `ZenInstance::from_raw_pointer()` in the Rust crate, replacing silent undefined behavior with clear panic messages. | ||
|
|
||
| ## Motivation | ||
|
|
||
| `from_raw_pointer` converts a raw C `ZenInstanceExtern*` pointer back into a Rust `&ZenInstance<T>` reference. Previously, passing a null pointer or receiving a misaligned custom-data pointer would cause undefined behavior (null dereference or misaligned access). The fix adds three `assert!` checks: | ||
|
|
||
| 1. `c_ptr` is not null | ||
| 2. The custom-data pointer returned by `ZenGetInstanceCustomData` is not null | ||
| 3. The custom-data pointer is properly aligned for `ZenInstance<T>` | ||
|
|
||
| ## Impact | ||
|
|
||
| - **Module**: `rust_crate/src/core/instance.rs` | ||
| - Only affects the Rust binding layer; no changes to the C++ core. | ||
| - Callers passing invalid pointers will now get a clear panic message instead of silent UB. Note: since the project uses `panic=abort` by default, panics at the FFI boundary will abort the process rather than unwinding across the FFI boundary (which would itself be UB). This ensures safe, deterministic failure. | ||
|
|
||
| ## Checklist | ||
|
|
||
| - [x] Implementation complete | ||
| - [x] Tests added/updated | ||
| - [ ] Module specs in `docs/modules/` updated (if affected) | ||
| - [x] Build and tests pass | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -71,10 +71,20 @@ impl<T> ZenInstance<T> { | |||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /// get rust ZenInstance from c instance* pointer | ||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||
| /// # Panics | ||||||||||||||||||||||||||||||||||||
| /// Panics if `c_ptr` is null or if the returned custom data pointer is | ||||||||||||||||||||||||||||||||||||
| /// null or misaligned. | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add change doc, and add tests which will cause the problem |
||||||||||||||||||||||||||||||||||||
| #[allow(clippy::not_unsafe_ptr_arg_deref)] | ||||||||||||||||||||||||||||||||||||
| pub fn from_raw_pointer(c_ptr: *mut ZenInstanceExtern) -> &'static ZenInstance<T> { | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+75
to
79
|
||||||||||||||||||||||||||||||||||||
| /// # Panics | |
| /// Panics if `c_ptr` is null or if the returned custom data pointer is | |
| /// null or misaligned. | |
| #[allow(clippy::not_unsafe_ptr_arg_deref)] | |
| pub fn from_raw_pointer(c_ptr: *mut ZenInstanceExtern) -> &'static ZenInstance<T> { | |
| /// # Safety | |
| /// The caller must ensure that: | |
| /// - `c_ptr` is a valid, non-null `ZenInstanceExtern` pointer; | |
| /// - the custom data stored in `c_ptr` is a valid, properly aligned pointer | |
| /// to a live `ZenInstance<T>`; | |
| /// - the pointed-to `ZenInstance<T>` outlives the returned reference `'a`. | |
| /// | |
| /// # Panics | |
| /// Panics if `c_ptr` is null or if the returned custom data pointer is | |
| /// null or misaligned. | |
| #[allow(clippy::not_unsafe_ptr_arg_deref)] | |
| pub unsafe fn from_raw_pointer<'a>(c_ptr: *mut ZenInstanceExtern) -> &'a ZenInstance<T> { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -161,3 +161,54 @@ mod tests { | |
| assert_eq!("get_host_number", import_func0_name); | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod from_raw_pointer_tests { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests currently cover only the |
||
| use crate::core::instance::ZenInstance; | ||
| use crate::core::r#extern::{ZenInstanceExtern, ZenSetInstanceCustomData}; | ||
| use crate::core::runtime::ZenRuntime; | ||
| use std::cell::RefCell; | ||
| use std::ptr; | ||
| use std::rc::Rc; | ||
| fn create_runtime() -> RefCell<Rc<ZenRuntime>> { | ||
| RefCell::new(ZenRuntime::new(None)) | ||
| } | ||
| /// Helper: create a real WASM instance and return its raw C pointer. | ||
| /// The returned pointer is valid for the lifetime of the returned | ||
| /// `ZenInstance`, which must be kept alive by the caller. | ||
| fn create_real_instance() -> Rc<ZenInstance<i64>> { | ||
| let rt = create_runtime(); | ||
| let wasm_path = "./example/fib.0.wasm"; | ||
| let wasm_mod = rt.borrow_mut().load_module(wasm_path).unwrap(); | ||
| let isolation = rt.borrow_mut().new_isolation().unwrap(); | ||
| let gas_limit: u64 = 100000000; | ||
| wasm_mod.new_instance(isolation, gas_limit).unwrap() | ||
| } | ||
| #[test] | ||
| #[should_panic(expected = "from_raw_pointer: c_ptr is null")] | ||
| fn test_from_raw_pointer_null_c_ptr_panics() { | ||
| let null_ptr: *mut ZenInstanceExtern = ptr::null_mut(); | ||
| let _inst: &ZenInstance<i64> = ZenInstance::from_raw_pointer(null_ptr); | ||
| } | ||
| #[test] | ||
| #[should_panic(expected = "from_raw_pointer: custom data pointer is null")] | ||
| fn test_from_raw_pointer_null_custom_data_panics() { | ||
| let inst = create_real_instance(); | ||
| let c_ptr = inst.ptr; | ||
| unsafe { | ||
| ZenSetInstanceCustomData(c_ptr, ptr::null()); | ||
| } | ||
| let _result: &ZenInstance<i64> = ZenInstance::from_raw_pointer(c_ptr); | ||
| } | ||
| #[test] | ||
| #[should_panic(expected = "from_raw_pointer: custom data pointer is misaligned")] | ||
| fn test_from_raw_pointer_misaligned_custom_data_panics() { | ||
| let inst = create_real_instance(); | ||
| let c_ptr = inst.ptr; | ||
| let misaligned_ptr = 0x1usize as *const cty::c_void; | ||
| unsafe { | ||
| ZenSetInstanceCustomData(c_ptr, misaligned_ptr); | ||
| } | ||
| let _result: &ZenInstance<i64> = ZenInstance::from_raw_pointer(c_ptr); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.