diff --git a/crates/pf-encode/src/enc/linux/vk_build.rs b/crates/pf-encode/src/enc/linux/vk_build.rs index 403f9678..f68f4459 100644 --- a/crates/pf-encode/src/enc/linux/vk_build.rs +++ b/crates/pf-encode/src/enc/linux/vk_build.rs @@ -17,7 +17,7 @@ // child-module shape. External imports are this file's own; `vk_util` is a crate-root sibling, // so the path is `crate::`, not the parent-relative `super::` the parent uses. use super::*; -use crate::vk_util::{find_mem, make_plain_image, make_view}; +use crate::vk_util::{ext_advertised, find_mem, make_plain_image, make_view}; use anyhow::{bail, Result}; use ash::vk; use std::ffi::c_void; @@ -53,10 +53,10 @@ pub(super) unsafe fn probe_rgb_direct( let Ok(exts) = instance.enumerate_device_extension_properties(pd) else { return Err("probe-failed(ext-enum)"); }; - if !exts - .iter() - .any(|e| std::ffi::CStr::from_ptr(e.extension_name.as_ptr()) == vrgb::EXTENSION_NAME) - { + // Route through `vk_util::ext_advertised` rather than open-coding the walk a second time: + // this copy used the same unbounded `CStr::from_ptr` and had the same read-past-the-array + // hazard on a driver that fills all VK_MAX_EXTENSION_NAME_SIZE bytes without a NUL. + if !ext_advertised(&exts, vrgb::EXTENSION_NAME) { return Err("no-ext(mesa<26.0-or-no-efc)"); } // 2. Feature bit. diff --git a/crates/pf-encode/src/enc/linux/vk_util.rs b/crates/pf-encode/src/enc/linux/vk_util.rs index e1455dac..17104607 100644 --- a/crates/pf-encode/src/enc/linux/vk_util.rs +++ b/crates/pf-encode/src/enc/linux/vk_util.rs @@ -19,11 +19,15 @@ use pf_frame::PixelFormat; /// barriers were used without the extension ever being enabled; `pf-presenter/dmabuf.rs` is the /// in-repo precedent that enables it). pub(super) fn ext_advertised(exts: &[vk::ExtensionProperties], name: &std::ffi::CStr) -> bool { - exts.iter().any(|e| { - // SAFETY: `extension_name` is a spec-guaranteed NUL-terminated UTF-8 byte array inside - // the driver-filled `VkExtensionProperties` (VK_MAX_EXTENSION_NAME_SIZE bound). - unsafe { std::ffi::CStr::from_ptr(e.extension_name.as_ptr()) == name } - }) + // `extension_name_as_c_str()` is ash's BOUNDED accessor: it stops at + // `VK_MAX_EXTENSION_NAME_SIZE` and returns `Err` when the array holds no NUL, so a + // malformed driver entry is a non-match rather than a read past the array. The previous + // `CStr::from_ptr(e.extension_name.as_ptr())` had no in-Rust bound at all — its SAFETY + // comment asserted the spec guarantee instead of enforcing it, so a driver that filled all + // 256 bytes without a terminator ran the walk into the NEXT `ExtensionProperties` and, on + // the last element, past the allocation. Same accessor `pyrowave.rs` already uses for the + // identical job. No unsafe, no unchecked read, same answer on every well-formed driver. + exts.iter().any(|e| e.extension_name_as_c_str() == Ok(name)) } pub(crate) fn color_range(layer: u32) -> vk::ImageSubresourceRange { @@ -453,6 +457,29 @@ mod tests { )); } + /// A driver entry with NO terminator anywhere in `extension_name` must be a non-match, not a + /// read past the array. + /// + /// This is the case the old `CStr::from_ptr(e.extension_name.as_ptr())` could not survive: + /// with every one of VK_MAX_EXTENSION_NAME_SIZE bytes non-NUL it walked into the NEXT + /// `ExtensionProperties`, and on the LAST element past the allocation entirely. The old test + /// only ever built well-formed, NUL-terminated entries, so it proved nothing about the bound + /// — which is why the hazard survived a SAFETY comment that asserted the spec guarantee + /// rather than enforcing it. + #[test] + fn ext_advertised_rejects_unterminated_name_without_overrunning() { + let mut bad = ash::vk::ExtensionProperties::default(); + bad.extension_name.fill(b'A' as std::ffi::c_char); + // Deliberately LAST, so an unbounded walk would leave the whole array. + let exts = [ash::vk::ExtensionProperties::default(), bad]; + assert!(!super::ext_advertised( + &exts, + ash::ext::queue_family_foreign::NAME + )); + // And a name that is a prefix of the garbage still must not match. + assert!(!super::ext_advertised(&exts, c"AAAA")); + } + use super::*; /// CSC mode (`bgra_target = false`): the 3→4 expand is a pure byte shuffle — no channel