Skip to content

Commit

Permalink
Fix unsoundness in new_library_with_data
Browse files Browse the repository at this point in the history
The `library_data` slice passed to the function that the dispatch data object is referencing does not necessarily outlive the library in which it will be contained.

(Note: It _looks_ like we're upholding memory management rules here, as the object returned from `dispatch_data_create` is released with `dispatch_release` before the end of the function, but remember that the dispatch data is a reference-counted object; `MTLLibrary` will retain the dispatch data beyond the lifetime of the function).

As specified in https://developer.apple.com/documentation/dispatch/1452970-dispatch_data_create, if we use DISPATCH_DATA_DESTRUCTOR_DEFAULT as the destructor block instead, `dispatch` will copy the buffer for us automatically.
  • Loading branch information
madsmtm authored and grovesNL committed May 23, 2024
1 parent 411c4d3 commit 6b7e39d
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions src/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use super::*;

use block::{Block, ConcreteBlock};
use block::Block;
use objc::runtime::{NO, YES};

use std::{ffi::CStr, os::raw::c_char, path::Path, ptr};
Expand Down Expand Up @@ -1471,6 +1471,8 @@ pub type dispatch_queue_t = *mut Object;
#[allow(non_camel_case_types)]
type dispatch_block_t = *const Block<(), ()>;

const DISPATCH_DATA_DESTRUCTOR_DEFAULT: dispatch_block_t = ptr::null();

#[cfg_attr(
all(feature = "link", any(target_os = "macos", target_os = "ios")),
link(name = "System", kind = "dylib")
Expand Down Expand Up @@ -1704,12 +1706,20 @@ impl DeviceRef {

pub fn new_library_with_data(&self, library_data: &[u8]) -> Result<Library, String> {
unsafe {
let destructor_block = ConcreteBlock::new(|| {}).copy();
// SAFETY:
// `library_data` does not necessarily outlive the dispatch data
// in which it will be contained (since the dispatch data will be
// contained in the MTLLibrary returned by this function).
//
// To prevent the MTLLibrary from referencing the data outside of
// its lifetime, we use DISPATCH_DATA_DESTRUCTOR_DEFAULT as the
// destructor block, which will make `dispatch_data_create` copy
// the buffer for us automatically.
let data = dispatch_data_create(
library_data.as_ptr() as *const std::ffi::c_void,
library_data.len() as crate::c_size_t,
&_dispatch_main_q as *const _ as dispatch_queue_t,
&*destructor_block.deref(),
DISPATCH_DATA_DESTRUCTOR_DEFAULT,
);

let library: *mut MTLLibrary = try_objc! { err =>
Expand Down

0 comments on commit 6b7e39d

Please sign in to comment.