Unsound unsafe code in vulkano_utils Context::new

[Fuzzyzilla]

• Version of vulkano: 0.33.0
• OS: N/A
• GPU (the selected PhysicalDevice): N/A
• GPU Driver: N/A
• Upload of a reasonably minimal complete main.rs file that demonstrates the issue: See below (GitHub won't let me upload a .rs file, for whatever reason)

Issue

vulkano_util's Context::new has an unsound unsafe block at context.rs line 154 - There is no check that the user's closure does not in fact make any calls to the vulkan API, and thus UB can be caused with purely safe code on the user side.

Minimal demonstration:

use std::sync::Arc;
static DEVICE_RW: std::sync::OnceLock<Arc<vulkano::device::Device>> =
    std::sync::OnceLock::<Arc<vulkano::device::Device>>::new();
fn main() {
    let config = vulkano_util::context::VulkanoConfig {
        instance_create_info: vulkano::instance::InstanceCreateInfo {
            enabled_extensions: vulkano::instance::InstanceExtensions {
                ext_debug_utils: true,
                ..Default::default()
            },
            ..Default::default()
        },
        debug_create_info: Some(
            vulkano::instance::debug::DebugUtilsMessengerCreateInfo::user_callback(Arc::new(|_| {
                if let Some(device) = DEVICE_RW.get() {
                    // We now have safe access to the vulkan api where we absolutely should not.
                    // Do crimes.
                }
            })),
        ),
        ..Default::default()
    };
    let context = vulkano_util::context::VulkanoContext::new(config);
    let device = context.device();

    // Give our device to the debug messenger
    let _ = DEVICE_RW.get_or_init(|| device.clone());

    // Do something to trigger a debug message, and behold as unsafety crimes occur.
    println!("Hello, World!");
}

[Rua]

Good catch!

[Rua]

I think fundamentally the issue is that Instance has two constructors, so Context also needs two constructors, which is annoying.

This could be solved by making a wrapper type around the callback function, that can only be constructed unsafely. The safety condition of that wrapper would be "the wrapped function must not call the Vulkan API". Then, the user must promise to follow that condition to construct the wrapper, and then all other parts of the API take that wrapper type and become safe. Essentially, the unsafety is shifted away from the construction of Instance, and onto the construction of the wrapper. Instance only needs one constructor then.