From b35479e087a758618d20dc5f6af0ec515e3b8797 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 23 Apr 2024 00:21:28 +0200 Subject: [PATCH] Fix unsoundness in `new_library_with_data` 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. --- src/device.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/device.rs b/src/device.rs index 75061cc..a8e7508 100644 --- a/src/device.rs +++ b/src/device.rs @@ -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}; @@ -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") @@ -1704,12 +1706,20 @@ impl DeviceRef { pub fn new_library_with_data(&self, library_data: &[u8]) -> Result { 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 =>