Skip to content
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

Investigate the soundness of tagged Objective-C objects behind references #235

Open
madsmtm opened this issue Aug 2, 2022 · 6 comments
Open
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates help wanted Extra attention is needed I-unsound A soundness hole

Comments

@madsmtm
Copy link
Owner

madsmtm commented Aug 2, 2022

I've spent a lot of time in the past ensuring that the usual stacked borrows rules are upheld (for example ensuring that &NSObject and &mut NSObject never coexist), but I haven't actually ensured that even creating the reference to &NSObject is sound in the first place!

Our current definition of all object types is roughly:

#[repr(C)]
struct NSObject {
    inner: std::cell::UnsafeCell<[u8; 0]>,
}

// In the future:
extern "C" {
    type NSObject;
}

That is, a ZST (zero-sized type) with UnsafeCell to mark it as mutable behind shared references (since we don't know how a specific instance is implemented).

This is then used roughly as follows:

struct SEL { ... } // Unimportant

/// Calls the `hash` selector on the object
fn call_hash_selector(obj: &NSObject) -> usize {
    extern "C" { // In the future: "C-unwind"
        fn objc_msgSend(obj: *const NSObject , sel: SEL) -> usize;
    }
    unsafe { objc_msgSend(obj, SEL::hash()) }
}

let obj: &NSObject; // Get `&NSObject` from Objective-C somehow
let hash = call_hash_selector(obj);

Importantly, we use a reference to NSObject. This means that we must uphold certain properties that raw pointers don't need to!

Reading the documentation, in particular the fact that references must be aligned and dereferenceable is concerning: It is common for Objective-C to use "tagged classes", which essentially means that &NSObject may be a tagged pointer and not an actual pointer (examples: NSString & NSNumber).

Currently, the fact that NSObject is a ZST makes rustc not output the dereferenceable LLVM attribute, but it still outputs align 1 which is problematic (of course, things needs to be allowed by the language, not just allowed by current LLVM output, but it's a useful metric).

Need to find a solution to this!

@madsmtm madsmtm added the help wanted Extra attention is needed label Aug 2, 2022
@madsmtm
Copy link
Owner Author

madsmtm commented Aug 2, 2022

Note that we never actually attempt to dereference tagged objects in Rust code, we only send them to Objective-C via. objc_msgSend, so it is unlikely that this will cause miscompilations in practice. But still!

@madsmtm
Copy link
Owner Author

madsmtm commented Aug 2, 2022

Perhaps extern { type T; } could be specified such that &T doesn't guarantee anything other than being non-null?

I suspect this pattern of wanting certain references to be able to store maybe-tagged pointers is not uncommon, e.g. the foreign-types would be just as unsound to use with types that may be tagged.

Need to open issue / search for similar at the unsafe code guidelines! EDIT: done

@madsmtm madsmtm changed the title Investigate the soundness of accessing Objective-C objects behind references Investigate the soundness of Objective-C objects behind references Aug 2, 2022
@madsmtm madsmtm changed the title Investigate the soundness of Objective-C objects behind references Investigate the soundness of tagged Objective-C objects behind references Aug 2, 2022
@madsmtm madsmtm added the A-framework Affects the framework crates and the translator for them label Aug 2, 2022
@madsmtm
Copy link
Owner Author

madsmtm commented Aug 8, 2022

Alternatively we change our entire API surface to use something like:

extern "C" {
    type Opaque;
}

pub struct NSObject<'a> {
    ptr: NonNull<Opaque>,
    p: PhantomData<&'a ()>,
}

// Usage
let n: Id<NSObject<'static>, Owned>;
let n_ref: &NSObject<'_> = &*n; // Deref
let n_mut: &mut NSObject<'_> = &mut *n; // DerefMut

extern "C" {
    fn my_fn(obj: NSObject<'a>) {} // What is the mutability of `obj` here???
}

Unsure how exactly this would work!

@madsmtm
Copy link
Owner Author

madsmtm commented Aug 8, 2022

We'd probably need NSObjectRef<'a> and NSObjectMut<'a> as separate types

@madsmtm
Copy link
Owner Author

madsmtm commented Nov 22, 2022

Related: Class, Ivar, Method, ... also assume that they're actual pointers (and as such cannot just be indices into a global table)

@madsmtm madsmtm added A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates and removed A-framework Affects the framework crates and the translator for them labels Jan 27, 2023
@madsmtm
Copy link
Owner Author

madsmtm commented Sep 11, 2023

I tried making an example that can be run under Miri, see this gist.

For now, it seems like Miri treats zero-sized types specially enough that patterns the ones we use is allowed (or, at least not disallowed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates help wanted Extra attention is needed I-unsound A soundness hole
Projects
None yet
Development

No branches or pull requests

1 participant