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

Jump to bottom

ServoMedia::get is unsound #306

Closed
nox opened this issue Sep 11, 2019 · 3 comments
Closed

ServoMedia::get is unsound #306

nox opened this issue Sep 11, 2019 · 3 comments

Comments

@nox
Copy link
Contributor

nox commented Sep 11, 2019

The initialization of ServoMedia is very much unsound and should just be using lazy_static.

This is INSTANCE, note how it is initialized as a raw null pointer (that should be using std::ptr::null_mut() btw):

media/servo-media/lib.rs

Line 25 in a70f024

static mut INSTANCE: *mut Mutex<Option<Arc<ServoMedia>>> = 0 as *mut _;

Those are the three methods handling initialization:

media/servo-media/lib.rs

Lines 68 to 90 in a70f024

impl ServoMedia {
pub fn init<B: BackendInit>() {
INITIALIZER.call_once(|| unsafe {
let instance = Arc::new(ServoMedia(B::init()));
INSTANCE = Box::into_raw(Box::new(Mutex::new(Some(instance))));
})
}
pub fn init_with_backend(backend: Box<dyn Backend>) {
INITIALIZER.call_once(|| unsafe {
let instance = Arc::new(ServoMedia(backend));
INSTANCE = Box::into_raw(Box::new(Mutex::new(Some(instance))));
})
}
pub fn get() -> Result<Arc<ServoMedia>, ()> {
let instance = unsafe { &*INSTANCE }.lock().unwrap();
match *instance {
Some(ref instance) => Ok(instance.clone()),
None => Err(()),
}
}
}

Note how calling ServoMedia::get() before ServoMedia::init() or ServoMedia::init_with_backend(backend) will dereference a null pointer.

Please use lazy_static instead of manually using Once and a raw pointer in a static.
@nox
Copy link
Contributor Author

nox commented Sep 11, 2019

The raw pointer and Once value was previously soundly used but it was broken when the method init was introduced.

@khodzha
Copy link
Contributor

khodzha commented Jan 26, 2020

How does one pass Box<dyn Backend> into lazy_static block?

@khodzha khodzha mentioned this issue Apr 13, 2020
Merged
@Manishearth
Copy link
Member

#335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nox @Manishearth @khodzha