Repository navigation
fix(other): add safety checks to from_raw_pointer - #478
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds additional runtime validation when converting a raw FFI instance pointer into a Rust ZenInstance, aiming to reduce UB at the FFI boundary (Issue H-10).
Changes:
- Add null-check for
c_ptrbefore calling into the FFI getter. - Add null-check + alignment assertion for the custom data pointer before dereferencing.
- Document panic behavior for invalid pointers/misalignment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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> { |
There was a problem hiding this comment.
from_raw_pointer returns a &'static ZenInstance<T> derived from an arbitrary raw pointer. This lifetime is not actually 'static (the ZenInstance can be dropped), so callers could accidentally hold onto the reference past the instance lifetime, leading to use-after-free UB. Consider changing the signature to return a reference with an explicit lifetime via an unsafe fn from_raw_pointer<'a>(...) -> &'a ZenInstance<T> (documenting the required validity), or return NonNull<ZenInstance<T>>/*const ZenInstance<T> and keep the unsafety at the call site.
Suggested change
| /// # 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> { |
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions |
zoowii
reviewed
Apr 16, 2026
| /// | ||
| /// # 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.
add change doc, and add tests which will cause the problem
cmgCr
reviewed
Apr 17, 2026
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod from_raw_pointer_tests { |
Contributor
There was a problem hiding this comment.
Tests currently cover only the c_ptr is null panic. The newly added custom data pointer is null and misaligned assertions are not explicitly tested, creating a coverage gap vs. commit intent.
ys8888john
force-pushed
the
fix-p2-h10-bug
branch
4 times, most recently
from
April 21, 2026 09:59
2b28b95 to
1b42dea
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ys8888john
force-pushed
the
fix-p2-h10-bug
branch
2 times, most recently
from
April 23, 2026 09:29
fa8c235 to
12989c0
Compare
ys8888john
force-pushed
the
fix-p2-h10-bug
branch
from
April 30, 2026 02:26
12989c0 to
1b66812
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add null-check for c_ptr, null-check for custom data pointer, and alignment assertion before dereferencing in from_raw_pointer. This prevents undefined behavior when called with invalid pointers at the FFI boundary.
Issue: H-10
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
6. Release note