-
Notifications
You must be signed in to change notification settings - Fork 47
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
More FFI #29
base: master
Are you sure you want to change the base?
More FFI #29
Conversation
make it thread-safe and also resistant to compiler optimization (setting the tag to invalid may be optimized away)
I have mixed feelings about this: to me not trusting the FFI language to overflow w.r.t to the given pointer is pushing things too far. Or to put it in another way: why should it be acceptable to send heap pointers to FFI if there is a risk of overflow from FFI? Although harder to exploit, heap overflows are also exploitable. At that level of mistrust FFI with shared memory should not be used altogether, and instead messages and some form of sandboxing ought to be used. I mean, an additional protection against overflows can be a nice thing to have (I just don't think that "not giving stack pointers to FFI" is the way to go), for instance, here is a a prototype: type Canary = [u8; 4];
#[repr(C)] // to ensure ordering
pub
struct Canaried<T> {
inner: T,
canary: Canary,
}
impl<T> From<T> for Canaried<T> {
fn from (inner: T) -> Self
{
Self { inner, canary: ::rand::random() }
}
}
impl<T> Into<T> for Canaried<T> { ... } //
impl<T> Canaried<T> {
pub
fn with_mut_ptr<R> (self: &'_ mut Self, f: impl FnOnce(*mut T) -> R) -> R
{
use ::std::panic;
let at_canary: *mut Canary = &mut self.canary;
let canary = unsafe { at_canary.read_volatile() };
let ret = panic::catch_unwind(panic::AssertUnwindSafe(|| f(&mut self.inner)));
if canary != unsafe { at_canary.read_volatile() } {
::std::process::abort();
}
unsafe {
at_canary.write_volatile(::rand::random());
}
ret.unwrap_or_else(|err| panic::resume_unwind(err))
}
} |
This reverts commit 68b2408.
Yeah it's questionable. I think at the very least it should be downgraded to a rec rather that a rule. I made it to be consistent with a comment -I made a long time ago- about not providing stack pointers to C in the example. The point of the example is in fact not to ensure stack smashing or overflow but rather how to ensure drop is called. For the overflow problem I like your example much better! I think I will revert the new rule, simplify the example and reopen an issue to discuss that... |
Some typos
Add a rule about not passing stack pointers to foreign languageImprove and fix C API callback-based example