search

gfx-rs / metal-rs

Rust bindings for Metal
Apache License 2.0
567 stars 112 forks source link

Fix unsoundness in `new_library_with_data` #309

Closed madsmtm closed 3 months ago

madsmtm commented 4 months ago

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.

Fixes https://github.com/gfx-rs/metal-rs/issues/308.

K0bin commented 4 months ago

Thanks for the quick fix!

MarijnS95 commented 4 months ago

@madsmtm how about adding a comment to the code with your findings? Specifically, explain above DISPATCH_DATA_DESTRUCTOR_DEFAULT, that this will create a copy of the passed data so that the dispatch data becomes independent of library_data: &[u8]? Could that be part of // Safety docs on unsafe for the dispatch_data_create() function?