Mercurial > crates > nonstick
diff src/libpam/memory.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 | 49d9e2b5c189 |
children | a508a69c068a |
line wrap: on
line diff
--- a/src/libpam/memory.rs Thu Jul 03 20:55:40 2025 -0400 +++ b/src/libpam/memory.rs Thu Jul 03 23:57:49 2025 -0400 @@ -1,41 +1,20 @@ //! Things for dealing with memory. +use crate::ErrorCode; use crate::Result; -use crate::{BinaryData, ErrorCode}; -use memoffset::offset_of; -use std::error::Error; +use libpam_sys_helpers::memory::{Buffer, OwnedBinaryPayload}; use std::ffi::{c_char, CStr, CString}; -use std::fmt::{Display, Formatter, Result as FmtResult}; use std::marker::{PhantomData, PhantomPinned}; use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; use std::ptr::NonNull; -use std::result::Result as StdResult; use std::{mem, ptr, slice}; -/// Raised from `calloc` when you have no memory! -#[derive(Debug)] -pub struct NoMem; - -impl Display for NoMem { - fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { - write!(f, "out of memory!") - } -} - -impl Error for NoMem {} - -impl From<NoMem> for ErrorCode { - fn from(_: NoMem) -> Self { - ErrorCode::BufferError - } -} - /// Allocates `count` elements to hold `T`. #[inline] -pub fn calloc<T>(count: usize) -> StdResult<NonNull<T>, NoMem> { +pub fn calloc<T>(count: usize) -> NonNull<T> { // SAFETY: it's always safe to allocate! Leaking memory is fun! - NonNull::new(unsafe { libc::calloc(count, mem::size_of::<T>()) }.cast()).ok_or(NoMem) + unsafe { NonNull::new_unchecked(libc::calloc(count, mem::size_of::<T>()).cast()) } } /// Wrapper for [`libc::free`] to make debugging calls/frees easier. @@ -79,7 +58,7 @@ impl<T> CHeapBox<T> { /// Creates a new CHeapBox holding the given data. pub fn new(value: T) -> Result<Self> { - let memory = calloc(1)?; + let memory = calloc(1); unsafe { ptr::write(memory.as_ptr(), value) } // SAFETY: We literally just allocated this. Ok(Self(memory)) @@ -107,6 +86,11 @@ this.0 } + /// Because it's annoying to type `CHeapBox.as_ptr(...).as_ptr()`. + pub fn as_raw_ptr(this: &Self) -> *mut T { + this.0.as_ptr() + } + /// Converts this into a Box of a different type. /// /// # Safety @@ -123,6 +107,31 @@ } } +impl Buffer for CHeapBox<u8> { + fn allocate(len: usize) -> Self { + // SAFETY: This is all freshly-allocated memory! + unsafe { Self::from_ptr(calloc(len)) } + } + + fn as_ptr(this: &Self) -> *const u8 { + this.0.as_ptr() + } + + unsafe fn as_mut_slice(this: &mut Self, len: usize) -> &mut [u8] { + slice::from_raw_parts_mut(this.0.as_ptr(), len) + } + + fn into_ptr(this: Self) -> NonNull<u8> { + CHeapBox::into_ptr(this) + } + + unsafe fn from_ptr(ptr: NonNull<u8>, _: usize) -> Self { + CHeapBox::from_ptr(ptr) + } +} + +pub type CHeapPayload = OwnedBinaryPayload<CHeapBox<u8>>; + impl<T> Deref for CHeapBox<T> { type Target = T; fn deref(&self) -> &Self::Target { @@ -164,10 +173,11 @@ return Err(ErrorCode::ConversationError); } // +1 for the null terminator - let data_alloc: NonNull<c_char> = calloc(data.len() + 1)?; + let data_alloc: NonNull<c_char> = calloc(data.len() + 1); // SAFETY: we just allocated this and we have enough room. unsafe { - libc::memcpy(data_alloc.as_ptr().cast(), data.as_ptr().cast(), data.len()); + let dest = slice::from_raw_parts_mut(data_alloc.as_ptr().cast(), data.len()); + dest.copy_from_slice(data); Ok(Self(CHeapBox::from_ptr(data_alloc))) } } @@ -248,101 +258,31 @@ Ok(borrowed.map(String::from)) } -/// Binary data used in requests and responses. -/// -/// This is an unsized data type whose memory goes beyond its data. -/// This must be allocated on the C heap. -/// -/// A Linux-PAM extension. -#[repr(C)] -pub struct CBinaryData { - /// The total length of the structure; a u32 in network byte order (BE). - total_length: [u8; 4], - /// A tag of undefined meaning. - data_type: u8, - /// Pointer to an array of length [`length`](Self::length) − 5 - data: [u8; 0], - _marker: Immovable, -} - -impl CBinaryData { - /// Copies the given data to a new BinaryData on the heap. - pub fn alloc((data, data_type): (&[u8], u8)) -> Result<CHeapBox<CBinaryData>> { - let buffer_size = - u32::try_from(data.len() + 5).map_err(|_| ErrorCode::ConversationError)?; - // SAFETY: We're only allocating here. - unsafe { - let mut dest_buffer: NonNull<Self> = calloc::<u8>(buffer_size as usize)?.cast(); - let dest = dest_buffer.as_mut(); - dest.total_length = buffer_size.to_be_bytes(); - dest.data_type = data_type; - libc::memcpy( - Self::data_ptr(dest_buffer).cast(), - data.as_ptr().cast(), - data.len(), - ); - Ok(CHeapBox::from_ptr(dest_buffer)) - } - } - - fn length(&self) -> usize { - u32::from_be_bytes(self.total_length).saturating_sub(5) as usize - } - - fn data_ptr(ptr: NonNull<Self>) -> *mut u8 { - unsafe { - ptr.as_ptr() - .cast::<u8>() - .byte_offset(offset_of!(Self, data) as isize) - } - } - - unsafe fn data_slice<'a>(ptr: NonNull<Self>) -> &'a mut [u8] { - unsafe { slice::from_raw_parts_mut(Self::data_ptr(ptr), ptr.as_ref().length()) } - } - - pub unsafe fn data<'a>(ptr: NonNull<Self>) -> (&'a [u8], u8) { - unsafe { (Self::data_slice(ptr), ptr.as_ref().data_type) } - } - - pub unsafe fn zero_contents(ptr: NonNull<Self>) { - for byte in Self::data_slice(ptr) { - ptr::write_volatile(byte as *mut u8, mem::zeroed()); - } - ptr::write_volatile(ptr.as_ptr(), mem::zeroed()); - } - - #[allow(clippy::wrong_self_convention)] - pub unsafe fn as_binary_data(ptr: NonNull<Self>) -> BinaryData { - let (data, data_type) = unsafe { (CBinaryData::data_slice(ptr), ptr.as_ref().data_type) }; - (Vec::from(data), data_type).into() - } -} - #[cfg(test)] mod tests { use super::*; - use std::hint; + use std::cell::Cell; #[test] fn test_box() { - #[allow(non_upper_case_globals)] - static mut drop_count: u32 = 0; + let drop_count: Cell<u32> = Cell::new(0); + + struct Dropper<'a>(&'a Cell<u32>); - struct Dropper(i32); - - impl Drop for Dropper { + impl Drop for Dropper<'_> { fn drop(&mut self) { - unsafe { drop_count += 1 } + self.0.set(self.0.get() + 1) } } - let mut dropbox = CHeapBox::new(Dropper(9)).unwrap(); - hint::black_box(dropbox.0); - dropbox = CHeapBox::new(Dropper(10)).unwrap(); - assert_eq!(1, unsafe { drop_count }); - hint::black_box(dropbox.0); + let mut dropbox = CHeapBox::new(Dropper(&drop_count)).unwrap(); + _ = dropbox; + // ensure the old value is dropped when the new one is assigned. + dropbox = CHeapBox::new(Dropper(&drop_count)).unwrap(); + assert_eq!(1, drop_count.get()); + *dropbox = Dropper(&drop_count); + assert_eq!(2, drop_count.get()); drop(dropbox); - assert_eq!(2, unsafe { drop_count }); + assert_eq!(3, drop_count.get()); } #[test] fn test_strings() {