Fix NULL-terminated strings in IOKit

When `const io_name_t` is used in arguments, it doesn't actually mean
that the length of the input must be 128 characters long; that part is
only a hint, the important thing is that the string is NULL-terminated.

(When it's used without `const`, it's an output, and then the length
_does_ matter. But that part we already handle correctly).

See discussion on Matrix for context:
https://matrix.to/#/!SrJvHgAPHenBakQHSz:matrix.org/$1VLn3ZskHtMuxGsP0TwPNnnLsNwTxxe_4bSs6WX1i_w?via=matrix.org&via=rymc.io&via=mozilla.org

Author: madsmtm

crates/header-translator/src/rust_type.rs

@@ -3015,10 +3015,20 @@ impl Ty {
         }
     }
 
+    /// Whether the type, if behind a pointer, is allowed to be converted
+    /// to/from `CStr`.
     fn is_pointee_cstr(&self) -> bool {
         // Only check `char`; `unsigned char` or `signed char` are not
         // converted to `CStr` automatically.
-        matches!(self.through_typedef(), Self::Primitive(Primitive::Char))
+        match self.through_typedef() {
+            Self::Primitive(Primitive::Char) => true,
+            // `[c_char; N]` arrays work similar to `*c_char` in that we want
+            // to map them to `&CStr` when possible.
+            Self::Array { element_type, .. } => {
+                matches!(**element_type, Self::Primitive(Primitive::Char))
+            }
+            _ => false,
+        }
     }
 
     pub(crate) fn contains_union(&self) -> bool {
@@ -3732,7 +3742,7 @@ impl Ty {
         })
     }
 
-    pub(crate) fn fn_argument(&self, allow_generic_param: bool) -> impl fmt::Display + '_ {
+    fn fn_argument(&self, allow_generic_param: bool) -> impl fmt::Display + '_ {
         FormatterFn(move |f| match self {
             Self::Pointer {
                 nullability,
@@ -3972,17 +3982,29 @@ impl Ty {
                 bounds: PointerBounds::NullTerminated,
                 pointee,
             } if pointee.is_pointee_cstr() => {
+                // We could emit a length check here for `char[N]`, to ensure
+                // that the user doesn't pass a string that is too long.
+                //
+                // But in practice, at least for the places that this is
+                // relevant (namely IOKit), it doesn't matter, the length is
+                // gotten via. `strlen`, and passing a string that is too long
+                // (seemingly) doesn't change the behaviour.
+
+                writeln!(f, "let {arg_to} = ")?;
                 if *nullability == Nullability::NonNull {
-                    writeln!(
-                        f,
-                        "let {arg_to} = NonNull::new({arg}.as_ptr().cast_mut()).unwrap();"
-                    )
+                    writeln!(f, "NonNull::new({arg}.as_ptr().cast_mut()).unwrap()")?;
                 } else {
                     writeln!(
                         f,
-                        "let {arg_to} = {arg}.map(|ptr| ptr.as_ptr()).unwrap_or_else(core::ptr::null);"
-                    )
+                        "{arg}.map(|ptr| ptr.as_ptr()).unwrap_or_else(core::ptr::null)"
+                    )?;
                 }
+                if !matches!(pointee.through_typedef(), Self::Primitive(Primitive::Char)) {
+                    writeln!(f, ".cast()")?;
+                }
+                writeln!(f, ";")?;
+
+                Ok(())
             }
             // HACK to support CFArray<T>.
             Self::Pointer {

framework-crates/objc2-io-kit/Cargo.modified.toml

@@ -0,0 +1,2 @@
+[dev-dependencies]
+objc2-core-foundation = { workspace = true, features = ["CFString"] }

framework-crates/objc2-io-kit/Cargo.toml

@@ -1,5 +1,6 @@
 #![allow(non_snake_case, clippy::missing_safety_doc)]
-use core::{ffi::c_void, ptr};
+use core::ffi::{c_void, CStr};
+use core::ptr;
 use objc2_core_foundation::{CFDictionary, CFRetained};
 
 use crate::{
@@ -23,6 +24,13 @@ fn consume(matching: Option<CFRetained<CFDictionary>>) -> *mut CFDictionary {
 /// Parameter `matching`: A CF dictionary containing matching information, of which one reference is always consumed by this function (Note prior to the Tiger release there was a small chance that the dictionary might not be released if there was an error attempting to serialize the dictionary). IOKitLib can construct matching dictionaries for common criteria with helper functions such as IOServiceMatching, IOServiceNameMatching, IOBSDNameMatching.
 ///
 /// Returns: The first service matched is returned on success. The service must be released by the caller.
+///
+/// # Safety
+///
+/// - `matching` generic must be of the correct type.
+/// - `matching` generic must be of the correct type.
+/// - `matching` might not allow `None`.
+#[inline]
 pub unsafe fn IOServiceGetMatchingService(
     main_port: libc::mach_port_t,
     matching: Option<CFRetained<CFDictionary>>,
@@ -48,6 +56,14 @@ pub unsafe fn IOServiceGetMatchingService(
 /// Parameter `existing`: An iterator handle, or NULL, is returned on success, and should be released by the caller when the iteration is finished. If NULL is returned, the iteration was successful but found no matching services.
 ///
 /// Returns: A kern_return_t error code.
+///
+/// # Safety
+///
+/// - `matching` generic must be of the correct type.
+/// - `matching` generic must be of the correct type.
+/// - `matching` might not allow `None`.
+/// - `existing` must be a valid pointer.
+#[inline]
 pub unsafe fn IOServiceGetMatchingServices(
     main_port: libc::mach_port_t,
     matching: Option<CFRetained<CFDictionary>>,
@@ -91,9 +107,21 @@ pub unsafe fn IOServiceGetMatchingServices(
 /// Parameter `notification`: An iterator handle is returned on success, and should be released by the caller when the notification is to be destroyed. The notification is armed when the iterator is emptied by calls to IOIteratorNext - when no more objects are returned, the notification is armed. Note the notification is not armed when first created.
 ///
 /// Returns: A kern_return_t error code.
+///
+/// # Safety
+///
+/// - `notify_port` must be a valid pointer.
+/// - `notification_type` might not allow `None`.
+/// - `matching` generic must be of the correct type.
+/// - `matching` generic must be of the correct type.
+/// - `matching` might not allow `None`.
+/// - `callback` must be implemented correctly.
+/// - `ref_con` must be a valid pointer.
+/// - `notification` must be a valid pointer.
+#[inline]
 pub unsafe fn IOServiceAddMatchingNotification(
     notify_port: IONotificationPortRef,
-    notification_type: *mut io_name_t,
+    notification_type: Option<&CStr>,
     matching: Option<CFRetained<CFDictionary>>,
     callback: IOServiceMatchingCallback,
     ref_con: *mut c_void,
@@ -102,14 +130,18 @@ pub unsafe fn IOServiceAddMatchingNotification(
     extern "C-unwind" {
         fn IOServiceAddMatchingNotification(
             notify_port: IONotificationPortRef,
-            notification_type: *mut io_name_t,
+            notification_type: *const io_name_t,
             matching: *mut CFDictionary,
             callback: IOServiceMatchingCallback,
             ref_con: *mut c_void,
             notification: *mut io_iterator_t,
         ) -> libc::kern_return_t;
     }
 
+    let notification_type = notification_type
+        .map(|ptr| ptr.as_ptr())
+        .unwrap_or_else(core::ptr::null)
+        .cast();
     unsafe {
         IOServiceAddMatchingNotification(
             notify_port,

framework-crates/objc2-io-kit/src/consumes_argument.rs

@@ -0,0 +1,95 @@
+#![cfg(feature = "libc")]
+
+use std::ffi::{CStr, CString};
+
+use libc::MACH_PORT_NULL;
+use objc2_io_kit::{
+    kIOReturnSuccess, kIOServicePlane, IOObjectConformsTo, IOObjectCopyClass, IOObjectGetClass,
+    IOObjectRelease, IORegistryEntryFromPath, IORegistryEntryGetPath, IORegistryGetRootEntry,
+};
+
+#[cfg(not(target_os = "macos"))]
+macro_rules! main_port {
+    () => {
+        #[allow(unused_unsafe)]
+        unsafe {
+            objc2_io_kit::kIOMainPortDefault
+        }
+    };
+}
+
+#[cfg(target_os = "macos")]
+macro_rules! main_port {
+    () => {{
+        #[allow(deprecated, unused_unsafe)]
+        unsafe {
+            objc2_io_kit::kIOMasterPortDefault
+        }
+    }};
+}
+
+#[test]
+fn out_pointer() {
+    let obj = IORegistryGetRootEntry(main_port!());
+
+    let mut name = [0; 128];
+    assert_eq!(
+        unsafe { IOObjectGetClass(obj, Some(&mut name)) },
+        kIOReturnSuccess
+    );
+    let name = name.map(|c| c as u8);
+    let name = CStr::from_bytes_until_nul(&name).unwrap();
+
+    let cf_name = IOObjectCopyClass(obj).unwrap();
+
+    assert_eq!(name.to_str().unwrap(), cf_name.to_string());
+    assert_eq!(name, c"IORegistryEntry");
+
+    assert_eq!(IOObjectRelease(obj), kIOReturnSuccess);
+}
+
+#[test]
+fn in_pointer() {
+    let obj = IORegistryGetRootEntry(main_port!());
+
+    assert!(unsafe { IOObjectConformsTo(obj, Some(c"IORegistryEntry")) });
+    assert!(!unsafe { IOObjectConformsTo(obj, Some(c"BogusClassName")) });
+
+    assert_eq!(IOObjectRelease(obj), kIOReturnSuccess);
+}
+
+#[test]
+fn entry_path() {
+    let path = c"IOService:/";
+
+    let obj = unsafe { IORegistryEntryFromPath(main_port!(), Some(path)) };
+    assert_ne!(obj, MACH_PORT_NULL as u32);
+
+    let mut out_path = [0; 512];
+    assert_eq!(
+        unsafe { IORegistryEntryGetPath(obj, Some(kIOServicePlane), Some(&mut out_path)) },
+        kIOReturnSuccess
+    );
+    let out_path = out_path.map(|c| c as u8);
+    let out_path = CStr::from_bytes_until_nul(&out_path).unwrap();
+    assert_eq!(out_path, path);
+
+    assert_eq!(IOObjectRelease(obj), kIOReturnSuccess);
+}
+
+#[test]
+fn entry_path_too_long() {
+    let path = CString::new([b'x'; 1000]).unwrap();
+    let obj = unsafe { IORegistryEntryFromPath(main_port!(), Some(&path)) };
+    assert_eq!(obj, MACH_PORT_NULL as u32);
+}
+
+#[test]
+fn in_pointer_too_long() {
+    let obj = IORegistryGetRootEntry(main_port!());
+
+    let name = CString::new([b'x'; 1000]).unwrap();
+    assert!(!unsafe { IOObjectConformsTo(obj, Some(&name)) });
+
+    assert_eq!(IOObjectRelease(obj), kIOReturnSuccess);
+}

framework-crates/objc2-io-kit/tests/strings.rs

@@ -251,6 +251,36 @@ fn.IOCFUnserialize.arguments.3.lifetime = "out-pointer-retained"
 fn.IOCFUnserializeBinary.arguments.4.lifetime = "out-pointer-retained"
 fn.IOCFUnserializeWithSize.arguments.4.lifetime = "out-pointer-retained"
 
+##
+## Pointer bounds
+##
+
+fn.IOObjectConformsTo.arguments.1.bounds = "null-terminated"
+fn.IOServiceAddNotification.arguments.1.bounds = "null-terminated"
+fn.IOServiceAddMatchingNotification.arguments.1.bounds = "null-terminated"
+fn.IOServiceAddInterestNotification.arguments.2.bounds = "null-terminated"
+fn.IORegistryEntryFromPath.arguments.1.bounds = "null-terminated"
+fn.IORegistryCreateIterator.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryCreateIterator.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetNameInPlane.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetLocationInPlane.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetPath.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryCopyPath.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntrySearchCFProperty.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetProperty.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetProperty.arguments.2.bounds = "single" # NOT sized by *size
+# fn.IORegistryEntryGetProperty.arguments.3.bounds = "single"
+fn.IORegistryEntryGetChildIterator.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetChildEntry.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetParentIterator.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryGetParentEntry.arguments.1.bounds = "null-terminated"
+fn.IORegistryEntryInPlane.arguments.1.bounds = "null-terminated"
+fn.IOServiceOFPathToBSDName.arguments.1.bounds = "null-terminated"
+
+# network
+fn.IONetworkSetPacketFiltersMask.arguments.1.bounds = "null-terminated"
+fn.IONetworkGetPacketFiltersMask.arguments.1.bounds = "null-terminated"
+
 ##
 ## Safety
 ##