From 6193575d1e7d337cd4125e3ad6b0d969d9f0cd86 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 16 Dec 2024 14:11:17 +0100 Subject: [PATCH] fix: Require non-null pointer in StorePath Fixes https://github.com/nixops4/nixops4/issues/65, possible undefined behavior. This doesn't make the code nice wrt *const/*mut distinction, but since we're not mutating it, this should be fine. (cherry picked from commit 75d448aad923a5f835f0562400e223df43103ea4) --- rust/nix-expr/src/eval_state.rs | 5 +++++ rust/nix-store/src/path.rs | 22 ++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/rust/nix-expr/src/eval_state.rs b/rust/nix-expr/src/eval_state.rs index 6e18d6a..46802d3 100644 --- a/rust/nix-expr/src/eval_state.rs +++ b/rust/nix-expr/src/eval_state.rs @@ -403,6 +403,11 @@ impl EvalState { let mut paths = Vec::with_capacity(n as usize); for i in 0..n { let path = raw::realised_string_get_store_path(rs, i); + let path = NonNull::new(path as *mut raw::StorePath).ok_or_else(|| { + anyhow::format_err!( + "nix_realised_string_get_store_path returned a null pointer" + ) + })?; paths.push(StorePath::new_raw_clone(path)); } paths diff --git a/rust/nix-store/src/path.rs b/rust/nix-store/src/path.rs index ad1a0f3..5fa847c 100644 --- a/rust/nix-store/src/path.rs +++ b/rust/nix-store/src/path.rs @@ -1,3 +1,5 @@ +use std::ptr::NonNull; + use anyhow::Result; use nix_c_raw as raw; use nix_util::{ @@ -6,38 +8,46 @@ use nix_util::{ }; pub struct StorePath { - raw: *mut raw::StorePath, + raw: NonNull, } impl StorePath { /// This is a low level function that you shouldn't have to call unless you are developing the Nix bindings. /// /// Construct a new `StorePath` by first cloning the C store path. /// This does not take ownership of the C store path, so it should be a borrowed value, or you should free it. - pub fn new_raw_clone(raw: *const raw::StorePath) -> Self { - Self::new_raw(unsafe { raw::store_path_clone(raw as *mut raw::StorePath) }) + pub fn new_raw_clone(raw: NonNull) -> Self { + Self::new_raw( + NonNull::new(unsafe { raw::store_path_clone(raw.as_ptr()) }) + .or_else(|| panic!("nix_store_path_clone returned a null pointer")) + .unwrap(), + ) } /// This is a low level function that you shouldn't have to call unless you are developing the Nix bindings. /// /// Takes ownership of a C `nix_store_path`. It will be freed when the `StorePath` is dropped. - pub fn new_raw(raw: *mut raw::StorePath) -> Self { + pub fn new_raw(raw: NonNull) -> Self { StorePath { raw } } pub fn name(&self) -> Result { unsafe { let mut r = result_string_init!(); raw::store_path_name( - self.raw, + self.as_ptr(), Some(callback_get_result_string), callback_get_result_string_data(&mut r), ); r } } + + pub fn as_ptr(&self) -> *mut nix_c_raw::StorePath { + self.raw.as_ptr() + } } impl Drop for StorePath { fn drop(&mut self) { unsafe { - raw::store_path_free(self.raw); + raw::store_path_free(self.as_ptr()); } } }