From 4ae4cfd0588fbe93e1cc6501bcb98893ebdf26f3 Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Wed, 23 Mar 2022 05:28:29 +0000 Subject: Make `uname` always safe This fixes several issues with the current `uname` bindings: - Do not ignore `uname` errors; at least on glibc `uname` can fail, so now it returns a `Result` instead of assuming that the call will always succeed. - Do not assume `uname` will initialize every member of `utsname`; not every implementation initializes every field, so internally the struct is now zero-initialized. - Do not blindly assume strings returned by `uname` will always be valid UTF-8; `UtsName`'s accessors will now return `&OsStr`s instead of `&str`s. --- CHANGELOG.md | 2 ++ src/features.rs | 22 +++++++++++---------- src/sys/utsname.rs | 56 ++++++++++++++++++++++++++++-------------------------- test/common/mod.rs | 14 +++++++------- 4 files changed, 50 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f52b5cdc..af84248e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,8 @@ This project adheres to [Semantic Versioning](https://semver.org/). - Deprecated `IpAddr`, `Ipv4Addr`, and `Ipv6Addr` in favor of their equivalents from the standard library. (#[1685](https://github.com/nix-rust/nix/pull/1685)) +- `uname` now returns a `Result` instead of blindly assuming the call never fails. +- Getters on the `UtsName` struct now return a `&OsStr` instead of `&str`. ### Fixed diff --git a/src/features.rs b/src/features.rs index ed80fd71..61080986 100644 --- a/src/features.rs +++ b/src/features.rs @@ -3,7 +3,9 @@ pub use self::os::*; #[cfg(any(target_os = "linux", target_os = "android"))] mod os { + use std::os::unix::ffi::OsStrExt; use crate::sys::utsname::uname; + use crate::Result; // Features: // * atomic cloexec on socket: 2.6.27 @@ -22,15 +24,15 @@ mod os { *dst += (b - b'0') as usize; } - fn parse_kernel_version() -> usize { - let u = uname(); + fn parse_kernel_version() -> Result { + let u = uname()?; let mut curr: usize = 0; let mut major: usize = 0; let mut minor: usize = 0; let mut patch: usize = 0; - for b in u.release().bytes() { + for &b in u.release().as_bytes() { if curr >= 3 { break; } @@ -50,7 +52,7 @@ mod os { } } - if major >= 3 { + Ok(if major >= 3 { VERS_3 } else if major >= 2 { if minor >= 7 { @@ -68,29 +70,29 @@ mod os { } } else { VERS_UNKNOWN - } + }) } - fn kernel_version() -> usize { + fn kernel_version() -> Result { static mut KERNEL_VERS: usize = 0; unsafe { if KERNEL_VERS == 0 { - KERNEL_VERS = parse_kernel_version(); + KERNEL_VERS = parse_kernel_version()?; } - KERNEL_VERS + Ok(KERNEL_VERS) } } /// Check if the OS supports atomic close-on-exec for sockets pub fn socket_atomic_cloexec() -> bool { - kernel_version() >= VERS_2_6_27 + kernel_version().map(|version| version >= VERS_2_6_27).unwrap_or(false) } #[test] pub fn test_parsing_kernel_version() { - assert!(kernel_version() > 0); + assert!(kernel_version().unwrap() > 0); } } diff --git a/src/sys/utsname.rs b/src/sys/utsname.rs index 98edee04..5bd3a539 100644 --- a/src/sys/utsname.rs +++ b/src/sys/utsname.rs @@ -1,8 +1,9 @@ //! Get system identification use std::mem; -use libc::{self, c_char}; -use std::ffi::CStr; -use std::str::from_utf8_unchecked; +use std::os::unix::ffi::OsStrExt; +use std::ffi::OsStr; +use libc::c_char; +use crate::{Errno, Result}; /// Describes the running system. Return type of [`uname`]. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] @@ -10,47 +11,48 @@ use std::str::from_utf8_unchecked; pub struct UtsName(libc::utsname); impl UtsName { - /// Name of the operating system implementation - pub fn sysname(&self) -> &str { - to_str(&(&self.0.sysname as *const c_char ) as *const *const c_char) + /// Name of the operating system implementation. + pub fn sysname(&self) -> &OsStr { + cast_and_trim(&self.0.sysname) } /// Network name of this machine. - pub fn nodename(&self) -> &str { - to_str(&(&self.0.nodename as *const c_char ) as *const *const c_char) + pub fn nodename(&self) -> &OsStr { + cast_and_trim(&self.0.nodename) } /// Release level of the operating system. - pub fn release(&self) -> &str { - to_str(&(&self.0.release as *const c_char ) as *const *const c_char) + pub fn release(&self) -> &OsStr { + cast_and_trim(&self.0.release) } /// Version level of the operating system. - pub fn version(&self) -> &str { - to_str(&(&self.0.version as *const c_char ) as *const *const c_char) + pub fn version(&self) -> &OsStr { + cast_and_trim(&self.0.version) } /// Machine hardware platform. - pub fn machine(&self) -> &str { - to_str(&(&self.0.machine as *const c_char ) as *const *const c_char) + pub fn machine(&self) -> &OsStr { + cast_and_trim(&self.0.machine) } } /// Get system identification -pub fn uname() -> UtsName { +pub fn uname() -> Result { unsafe { - let mut ret = mem::MaybeUninit::uninit(); - libc::uname(ret.as_mut_ptr()); - UtsName(ret.assume_init()) + let mut ret = mem::MaybeUninit::zeroed(); + Errno::result(libc::uname(ret.as_mut_ptr()))?; + Ok(UtsName(ret.assume_init())) } } -#[inline] -fn to_str<'a>(s: *const *const c_char) -> &'a str { - unsafe { - let res = CStr::from_ptr(*s).to_bytes(); - from_utf8_unchecked(res) - } +fn cast_and_trim(slice: &[c_char]) -> &OsStr { + let length = slice.iter().position(|&byte| byte == 0).unwrap_or(slice.len()); + let bytes = unsafe { + std::slice::from_raw_parts(slice.as_ptr().cast(), length) + }; + + OsStr::from_bytes(bytes) } #[cfg(test)] @@ -58,18 +60,18 @@ mod test { #[cfg(target_os = "linux")] #[test] pub fn test_uname_linux() { - assert_eq!(super::uname().sysname(), "Linux"); + assert_eq!(super::uname().unwrap().sysname(), "Linux"); } #[cfg(any(target_os = "macos", target_os = "ios"))] #[test] pub fn test_uname_darwin() { - assert_eq!(super::uname().sysname(), "Darwin"); + assert_eq!(super::uname().unwrap().sysname(), "Darwin"); } #[cfg(target_os = "freebsd")] #[test] pub fn test_uname_freebsd() { - assert_eq!(super::uname().sysname(), "FreeBSD"); + assert_eq!(super::uname().unwrap().sysname(), "FreeBSD"); } } diff --git a/test/common/mod.rs b/test/common/mod.rs index 84a0b4fa..544a1ae3 100644 --- a/test/common/mod.rs +++ b/test/common/mod.rs @@ -111,15 +111,15 @@ cfg_if! { let version_requirement = VersionReq::parse($version_requirement) .expect("Bad match_version provided"); - let uname = nix::sys::utsname::uname(); - println!("{}", uname.sysname()); - println!("{}", uname.nodename()); - println!("{}", uname.release()); - println!("{}", uname.version()); - println!("{}", uname.machine()); + let uname = nix::sys::utsname::uname().unwrap(); + println!("{}", uname.sysname().to_str().unwrap()); + println!("{}", uname.nodename().to_str().unwrap()); + println!("{}", uname.release().to_str().unwrap()); + println!("{}", uname.version().to_str().unwrap()); + println!("{}", uname.machine().to_str().unwrap()); // Fix stuff that the semver parser can't handle - let fixed_release = &uname.release().to_string() + let fixed_release = &uname.release().to_str().unwrap().to_string() // Fedora 33 reports version as 4.18.el8_2.x86_64 or // 5.18.200-fc33.x86_64. Remove the underscore. .replace("_", "-") -- cgit v1.2.3