crates/nonstick: libpam-sys/libpam-sys-helpers/src/memory.rs comparison

comparison libpam-sys/libpam-sys-helpers/src/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	efbc235f01d3
children	add7228adb2f

comparison

equal deleted inserted replaced

-:999bf07efbcb
+:33b9622ed6d2
 //! Helpers to deal with annoying memory management in the PAM API.
-//!
-//!
 use std::error::Error;
 use std::marker::{PhantomData, PhantomPinned};
 use std::mem::ManuallyDrop;
 use std::ptr::NonNull;
-use std::{any, fmt, mem, slice};
+use std::{any, fmt, mem, ptr, slice};
 /// A pointer-to-pointer-to-message container for PAM's conversation callback.
 ///
 /// The PAM conversation callback requires a pointer to a pointer of
 /// `pam_message`s. Linux-PAM handles this differently than all other
 unsafe impl<T> Sync for PtrPtrVec<T> where Vec<T>: Sync {}
 impl<T> PtrPtrVec<T> {
 /// Takes ownership of the given Vec and creates a vec of pointers to it.
 pub fn new(data: Vec<T>) -> Self {
-let pointers: Vec<_> = data.iter().map(|r| r as *const T).collect();
+let start = data.as_ptr();
+// We do this slightly tricky little dance to satisfy Miri:
+//
+// A pointer extracted from a reference can only legally access
+// that reference's memory. This means that if we say:
+//     pointers[0] = &data[0] as *const T;
+// we can't traverse through pointers[0] to reach data[1],
+// we can only use pointers[1].
+//
+// However, if we use the start-of-vec pointer from the `data` vector,
+// its "provenance"* is valid for the entire array (even if the address
+// of the pointer is the same). This avoids some behavior which is
+// technically undefined. While the CPU sees no difference between
+// those two pointers, the compiler is allowed to make optimizations
+// based on that provenance (even if, in this case, it isn't likely
+// to do so).
+//
+//       data.as_ptr() points here, and is valid for the whole Vec.
+//       ┃
+//       ┠─────────────────╮
+//       ┌─────┬─────┬─────┐
+//  data │ [0] │ [1] │ [2] │
+//       └─────┴─────┴─────┘
+//       ┠─────╯     ┊
+//       ┃     ┊     ┊
+//       (&data[0] as *const T) points to the same place, but is valid
+//       only for that 0th element.
+//             ┊     ┊
+//             ┠─────╯
+//             ┃
+//             (&data[1] as *const T) points here, and is only valid
+//             for that element.
+//
+// We only have to do this for pointers[0] because only that pointer
+// is used for accessing elements other than data[0] (in XSSO).
+//
+// * "provenance" is kind of like if every pointer in your program
+// remembered where it came from and, based on that, it had an implied
+// memory range it was valid for, separate from its address.
+// https://doc.rust-lang.org/std/ptr/#provenance
+// (It took a long time for me to understand this.)
+let mut pointers = Vec::with_capacity(data.len());
+// Ensure the 0th pointer has provenance from the entire vec
+// (even though it's numerically identical to &data[0] as *const T).
+pointers.push(start);
+// The 1st and everything thereafter only need to have the provenance
+// of their own memory.
+pointers.extend(data[1..].iter().map(|r| r as *const T));
 Self { data, pointers }
 }
 /// Gives you back your Vec.
 pub fn into_inner(self) -> Vec<T> {
 /// This is intended to allow you to bring your own allocator for
 /// [`OwnedBinaryPayload`]s.
 ///
 /// For an implementation example, see the implementation of this trait
 /// for [`Vec`].
-pub trait Buffer<T: Default> {
+pub trait Buffer {
 /// Allocates a buffer of `len` elements, filled with the default.
 fn allocate(len: usize) -> Self;
-fn as_ptr(&self) -> *const T;
+fn as_ptr(this: &Self) -> *const u8;
 /// Returns a slice view of `size` elements of the given memory.
 ///
 /// # Safety
 ///
 /// The caller must not request more elements than are allocated.
-unsafe fn as_mut_slice(&mut self, len: usize) -> &mut [T];
+unsafe fn as_mut_slice(this: &mut Self, len: usize) -> &mut [u8];
 /// Consumes this ownership and returns a pointer to the start of the arena.
-fn into_ptr(self) -> NonNull<T>;
+fn into_ptr(this: Self) -> NonNull<u8>;
 /// "Adopts" the memory at the given pointer, taking it under management.
 ///
 /// Running the operation:
 ///
 /// ```
 /// # use libpam_sys_helpers::memory::Buffer;
-/// # fn test<T: Default, OwnerType: Buffer<T>>(bytes: usize) {
+/// # fn test<T: Default, OwnerType: Buffer>(bytes: usize) {
 /// let owner = OwnerType::allocate(bytes);
-/// let ptr = owner.into_ptr();
+/// let ptr = OwnerType::into_ptr(owner);
 /// let owner = unsafe { OwnerType::from_ptr(ptr, bytes) };
 /// # }
 /// ```
 ///
 /// must be a no-op.
 ///
 /// # Safety
 ///
 /// The pointer must be valid, and the caller must provide the exact size
 /// of the given arena.
-unsafe fn from_ptr(ptr: NonNull<T>, bytes: usize) -> Self;
+unsafe fn from_ptr(ptr: NonNull<u8>, bytes: usize) -> Self;
 }
-impl<T: Default> Buffer<T> for Vec<T> {
+impl Buffer for Vec<u8> {
 fn allocate(bytes: usize) -> Self {
-(0..bytes).map(|_| Default::default()).collect()
+vec![0; bytes]
 }
-fn as_ptr(&self) -> *const T {
+fn as_ptr(this: &Self) -> *const u8 {
-Vec::as_ptr(self)
+Vec::as_ptr(this)
 }
-unsafe fn as_mut_slice(&mut self, bytes: usize) -> &mut [T] {
+unsafe fn as_mut_slice(this: &mut Self, bytes: usize) -> &mut [u8] {
-debug_assert!(bytes <= self.len());
+&mut this[..bytes]
-Vec::as_mut(self)
+}
-}
+fn into_ptr(this: Self) -> NonNull<u8> {
-fn into_ptr(self) -> NonNull<T> {
+let mut me = ManuallyDrop::new(this);
-let mut me = ManuallyDrop::new(self);
 // SAFETY: a Vec is guaranteed to have a nonzero pointer.
 unsafe { NonNull::new_unchecked(me.as_mut_ptr()) }
 }
-unsafe fn from_ptr(ptr: NonNull<T>, bytes: usize) -> Self {
+unsafe fn from_ptr(ptr: NonNull<u8>, bytes: usize) -> Self {
 Vec::from_raw_parts(ptr.as_ptr(), bytes, bytes)
 }
 }
 /// The structure of the "binary message" payload for the `PAM_BINARY_PROMPT`
 /// Fills in the provided buffer with the given data.
 ///
 /// This uses [`copy_from_slice`](slice::copy_from_slice) internally,
 /// so `buf` must be exactly 5 bytes longer than `data`, or this function
 /// will panic.
-pub fn fill(buf: &mut [u8], data_type: u8, data: &[u8]) {
+pub fn fill(buf: &mut [u8], data: &[u8], data_type: u8) {
 let ptr: *mut Self = buf.as_mut_ptr().cast();
 // SAFETY: We're given a slice, which always has a nonzero pointer.
 let me = unsafe { ptr.as_mut().unwrap_unchecked() };
 me.total_bytes_u32be = u32::to_be_bytes(buf.len() as u32);
 me.data_type = data_type;
 buf[5..].copy_from_slice(data)
 }
 /// The total storage needed for the message, including header.
-pub fn total_bytes(&self) -> usize {
+pub unsafe fn total_bytes(this: *const Self) -> usize {
-u32::from_be_bytes(self.total_bytes_u32be) as usize
+let header = this.as_ref().unwrap_unchecked();
+u32::from_be_bytes(header.total_bytes_u32be) as usize
 }
 /// Gets the total byte buffer of the BinaryMessage stored at the pointer.
 ///
 /// The returned data slice is borrowed from where the pointer points to.
 /// # Safety
 ///
 /// - The pointer must point to a valid `BinaryPayload`.
 /// - The borrowed data must not outlive the pointer's validity.
 pub unsafe fn buffer_of<'a>(ptr: *const Self) -> &'a [u8] {
-let header: &Self = ptr.as_ref().unwrap_unchecked();
+slice::from_raw_parts(ptr.cast(), Self::total_bytes(ptr).max(5))
-slice::from_raw_parts(ptr.cast(), header.total_bytes().max(5))
 }
 /// Gets the contents of the BinaryMessage stored at the given pointer.
 ///
 /// The returned data slice is borrowed from where the pointer points to.
 ///
 /// # Safety
 ///
 /// - The pointer must point to a valid `BinaryPayload`.
 /// - The borrowed data must not outlive the pointer's validity.
-pub unsafe fn contents<'a>(ptr: *const Self) -> (u8, &'a [u8]) {
+pub unsafe fn contents<'a>(ptr: *const Self) -> (&'a [u8], u8) {
 let header: &Self = ptr.as_ref().unwrap_unchecked();
-(header.data_type, &Self::buffer_of(ptr)[5..])
+(&Self::buffer_of(ptr)[5..], header.data_type)
+}
+/// Zeroes out the data of this payload.
+///
+/// # Safety
+///
+/// - The pointer must point to a valid `BinaryPayload`.
+/// - The binary payload must not be used in the future,
+///   since its length metadata is gone and so its buffer is unknown.
+pub unsafe fn zero(ptr: *mut Self) {
+let size = Self::total_bytes(ptr);
+let ptr: *mut u8 = ptr.cast();
+for x in 0..size {
+ptr::write_volatile(ptr.byte_add(x), mem::zeroed())
+}
 }
 }
 /// A binary message owned by some storage.
 ///
 /// This is an owned, memory-managed version of [`BinaryPayload`].
 /// The `O` type manages the memory where the payload lives.
 /// [`Vec<u8>`] is one such manager and can be used when ownership
 /// of the data does not need to transit through PAM.
 #[derive(Debug)]
-pub struct OwnedBinaryPayload<Owner: Buffer<u8>>(Owner);
+pub struct OwnedBinaryPayload<Owner: Buffer>(Owner);
-impl<O: Buffer<u8>> OwnedBinaryPayload<O> {
+impl<O: Buffer> OwnedBinaryPayload<O> {
 /// Allocates a new OwnedBinaryPayload.
 ///
 /// This will return a [`TooBigError`] if you try to allocate too much
 /// (more than [`BinaryPayload::MAX_SIZE`]).
-pub fn new(data_type: u8, data: &[u8]) -> Result<Self, TooBigError> {
+pub fn new(data: &[u8], type_: u8) -> Result<Self, TooBigError> {
 let total_len: u32 = (data.len() + 5).try_into().map_err(|_| TooBigError {
 size: data.len(),
 max: BinaryPayload::MAX_SIZE,
 })?;
 let total_len = total_len as usize;
 let mut buf = O::allocate(total_len);
 // SAFETY: We just allocated this exact size.
-BinaryPayload::fill(unsafe { buf.as_mut_slice(total_len) }, data_type, data);
+BinaryPayload::fill(
+unsafe { Buffer::as_mut_slice(&mut buf, total_len) },
+data,
+type_,
+);
 Ok(Self(buf))
 }
 /// The contents of the buffer.
-pub fn contents(&self) -> (u8, &[u8]) {
+pub fn contents(&self) -> (&[u8], u8) {
 unsafe { BinaryPayload::contents(self.as_ptr()) }
 }
 /// The total bytes needed to store this, including the header.
 pub fn total_bytes(&self) -> usize {
-unsafe { BinaryPayload::buffer_of(self.0.as_ptr().cast()).len() }
+unsafe { BinaryPayload::buffer_of(Buffer::as_ptr(&self.0).cast()).len() }
 }
 /// Unwraps this into the raw storage backing it.
 pub fn into_inner(self) -> O {
 self.0
 }
 /// Gets a const pointer to the start of the message's buffer.
 pub fn as_ptr(&self) -> *const BinaryPayload {
-self.0.as_ptr().cast()
+Buffer::as_ptr(&self.0).cast()
 }
 /// Consumes ownership of this message and converts it to a raw pointer
 /// to the start of the message.
 ///
 /// To clean this up, you should eventually pass it into [`Self::from_ptr`]
 /// with the same `O` ownership type.
 pub fn into_ptr(self) -> NonNull<BinaryPayload> {
-self.0.into_ptr().cast()
+Buffer::into_ptr(self.0).cast()
 }
 /// Takes ownership of the given pointer.
 ///
 /// # Safety
 ///
 /// You must provide a valid pointer, allocated by (or equivalent to one
 /// allocated by) [`Self::new`]. For instance, passing a pointer allocated
 /// by `malloc` to `OwnedBinaryPayload::<Vec<u8>>::from_ptr` is not allowed.
 pub unsafe fn from_ptr(ptr: NonNull<BinaryPayload>) -> Self {
-Self(O::from_ptr(ptr.cast(), ptr.as_ref().total_bytes()))
+Self(O::from_ptr(ptr.cast(), BinaryPayload::total_bytes(ptr.as_ptr())))
 }
 }
 #[cfg(test)]
 mod tests {
 #[test]
 fn test_binary_payload() {
 let simple_message = &[0u8, 0, 0, 16, 0xff, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
 let empty = &[0u8; 5];
-assert_eq!((0xff, &[0u8, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10][..]), unsafe {
+assert_eq!((&[0u8, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10][..], 0xff), unsafe {
 BinaryPayload::contents(simple_message.as_ptr().cast())
 });
-assert_eq!((0x00, &[][..]), unsafe {
+assert_eq!((&[][..], 0x00), unsafe {
 BinaryPayload::contents(empty.as_ptr().cast())
 });
 }
 #[test]
 fn test_owned_binary_payload() {
-let (typ, data) = (
+let (data, typ) = (
+&[0, 1, 1, 8, 9, 9, 9, 8, 8, 1, 9, 9, 9, 1, 1, 9, 7, 2, 5, 3][..],
 112,
-&[0, 1, 1, 8, 9, 9, 9, 8, 8, 1, 9, 9, 9, 1, 1, 9, 7, 2, 5, 3][..],
 );
-let payload = VecPayload::new(typ, data).unwrap();
+let payload = VecPayload::new(data, typ).unwrap();
-assert_eq!((typ, data), payload.contents());
+assert_eq!((data, typ), payload.contents());
 let ptr = payload.into_ptr();
 let payload = unsafe { VecPayload::from_ptr(ptr) };
-assert_eq!((typ, data), payload.contents());
+assert_eq!((data, typ), payload.contents());
 }
 #[test]
 #[ignore]
 fn test_owned_too_big() {
 assert_eq!(
 TooBigError {
 max: 0xffff_fffa,
 size: 0x1_0000_0001
 },
-VecPayload::new(5, &data).unwrap_err()
+VecPayload::new(&data, 5).unwrap_err()
 )
 }
 #[cfg(debug_assertions)]
 #[test]

Mercurial > crates > nonstick

comparison libpam-sys/libpam-sys-helpers/src/memory.rs @ 139:33b9622ed6d2