diff src/libpam/answer.rs @ 139:33b9622ed6d2

Remove redundant memory management in nonstick::libpam; fix UB. - Uses the libpam-sys-helpers BinaryPayload / OwnedBinaryPayload structs to handle memory management and parsing for Linux-PAM binary messages. - Gets rid of the (technically) undefined behavior in PtrPtrVec due to pointer provenance. - Don't check for malloc failing. It won't, even if it does. - Formatting/cleanups/etc.
author Paul Fisher <paul@pfish.zone>
date Thu, 03 Jul 2025 23:57:49 -0400
parents 80c07e5ab22f
children ebb71a412b58
line wrap: on
line diff
--- a/src/libpam/answer.rs	Thu Jul 03 20:55:40 2025 -0400
+++ b/src/libpam/answer.rs	Thu Jul 03 23:57:49 2025 -0400
@@ -2,8 +2,9 @@
 
 use crate::libpam::conversation::OwnedExchange;
 use crate::libpam::memory;
-use crate::libpam::memory::{CBinaryData, CHeapBox, CHeapString, Immovable};
+use crate::libpam::memory::{CHeapBox, CHeapPayload, CHeapString, Immovable};
 use crate::{ErrorCode, Result};
+use libpam_sys_helpers::memory::BinaryPayload;
 use std::ffi::{c_int, c_void, CStr};
 use std::mem::ManuallyDrop;
 use std::ops::{Deref, DerefMut};
@@ -23,7 +24,7 @@
     /// Builds an Answers out of the given answered Message Q&As.
     pub fn build(value: Vec<OwnedExchange>) -> Result<Self> {
         let mut outputs = Self {
-            base: memory::calloc(value.len())?,
+            base: memory::calloc(value.len()),
             count: value.len(),
         };
         // Even if we fail during this process, we still end up freeing
@@ -193,21 +194,22 @@
     ///
     /// The referenced data is copied to the C heap.
     /// We do not take ownership of the original data.
-    pub fn fill(dest: &mut Answer, data_and_type: (&[u8], u8)) -> Result<()> {
-        let allocated = CBinaryData::alloc(data_and_type)?;
-        let _ = dest.data.replace(unsafe { CHeapBox::cast(allocated) });
+    pub fn fill(dest: &mut Answer, (data, type_): (&[u8], u8)) -> Result<()> {
+        let payload = CHeapPayload::new(data, type_).map_err(|_| ErrorCode::BufferError)?;
+        let _ = dest
+            .data
+            .replace(unsafe { CHeapBox::cast(payload.into_inner()) });
         Ok(())
     }
 
     /// Gets the binary data in this answer.
-    pub fn data(&self) -> Option<NonNull<CBinaryData>> {
+    pub fn contents(&self) -> Option<(&[u8], u8)> {
         // SAFETY: We either got this data from PAM or allocated it ourselves.
         // Either way, we trust that it is either valid data or null.
         self.0
             .data
             .as_ref()
-            .map(CHeapBox::as_ptr)
-            .map(NonNull::cast)
+            .map(|data| unsafe { BinaryPayload::contents(CHeapBox::as_ptr(data).cast().as_ptr()) })
     }
 
     /// Zeroes out the answer data, frees it, and points our data to `null`.
@@ -217,10 +219,12 @@
     /// but it will clear potentially sensitive data.
     pub fn zero_contents(&mut self) {
         // SAFETY: We know that our data pointer is either valid or null.
-        // Once we're done, it's null and the answer is safe.
-        unsafe {
-            if let Some(ptr) = self.0.data.as_ref() {
-                CBinaryData::zero_contents(CHeapBox::as_ptr(ptr).cast())
+        if let Some(data) = self.0.data.as_mut() {
+            unsafe {
+                let total = BinaryPayload::total_bytes(CHeapBox::as_ptr(data).cast().as_ref());
+                let data: &mut [u8] =
+                    slice::from_raw_parts_mut(CHeapBox::as_raw_ptr(data).cast(), total);
+                data.fill(0)
             }
         }
     }
@@ -290,13 +294,9 @@
 
         if let [bin, radio] = &mut answers[..] {
             let up = unsafe { BinaryAnswer::upcast(bin) };
-            assert_eq!(BinaryData::from((&[1, 2, 3][..], 99)), unsafe {
-                CBinaryData::as_binary_data(up.data().unwrap())
-            });
+            assert_eq!((&[1, 2, 3][..], 99), up.contents().unwrap());
             up.zero_contents();
-            assert_eq!(BinaryData::default(), unsafe {
-                CBinaryData::as_binary_data(up.data().unwrap())
-            });
+            assert_eq!((&[][..], 0), up.contents().unwrap());
 
             assert_text_answer("beep boop", radio);
         } else {
@@ -323,9 +323,7 @@
         let real_data = BinaryData::new([1, 2, 3, 4, 5, 6, 7, 8], 9);
         BinaryAnswer::fill(&mut answer, (&real_data).into()).expect("alloc should succeed");
         let bin_answer = unsafe { BinaryAnswer::upcast(&mut answer) };
-        assert_eq!(real_data, unsafe {
-            CBinaryData::as_binary_data(bin_answer.data().unwrap())
-        });
+        assert_eq!(real_data, bin_answer.contents().unwrap().into());
     }
 
     #[test]