Mercurial > crates > nonstick
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]