diff src/libpam/question.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 a508a69c068a
line wrap: on
line diff
--- a/src/libpam/question.rs	Thu Jul 03 20:55:40 2025 -0400
+++ b/src/libpam/question.rs	Thu Jul 03 23:57:49 2025 -0400
@@ -2,13 +2,15 @@
 
 #[cfg(feature = "linux-pam-ext")]
 use crate::conv::{BinaryQAndA, RadioQAndA};
+use libpam_sys_helpers::memory::{BinaryPayload, TooBigError};
 use crate::conv::{ErrorMsg, Exchange, InfoMsg, MaskedQAndA, QAndA};
 use crate::libpam::conversation::OwnedExchange;
-use crate::libpam::memory::{CBinaryData, CHeapBox, CHeapString};
+use crate::libpam::memory::{CHeapBox, CHeapPayload, CHeapString};
 use crate::ErrorCode;
 use crate::Result;
 use num_enum::{IntoPrimitive, TryFromPrimitive};
 use std::ffi::{c_int, c_void, CStr};
+use std::ptr::NonNull;
 
 mod style_const {
     pub use libpam_sys::*;
@@ -46,7 +48,7 @@
 /// A question sent by PAM or a module to an application.
 ///
 /// PAM refers to this as a "message", but we call it a question
-/// to avoid confusion with [`Message`](crate::conv::Exchange).
+/// to avoid confusion.
 ///
 /// This question, and its internal data, is owned by its creator
 /// (either the module or PAM itself).
@@ -60,7 +62,7 @@
     /// For most requests, this will be an owned [`CStr`],
     /// but for requests with style `PAM_BINARY_PROMPT`,
     /// this will be `CBinaryData` (a Linux-PAM extension).
-    pub data: Option<CHeapBox<c_void>>,
+    pub data: Option<NonNull<c_void>>,
 }
 
 impl Question {
@@ -72,7 +74,7 @@
     unsafe fn string_data(&self) -> Result<&str> {
         match self.data.as_ref() {
             None => Ok(""),
-            Some(data) => CStr::from_ptr(CHeapBox::as_ptr(data).cast().as_ptr())
+            Some(data) => CStr::from_ptr(data.as_ptr().cast())
                 .to_str()
                 .map_err(|_| ErrorCode::ConversationError),
         }
@@ -82,7 +84,7 @@
     unsafe fn binary_data(&self) -> (&[u8], u8) {
         self.data
             .as_ref()
-            .map(|data| CBinaryData::data(CHeapBox::as_ptr(data).cast()))
+            .map(|data| BinaryPayload::contents(data.as_ptr().cast()))
             .unwrap_or_default()
     }
 }
@@ -104,9 +106,11 @@
             #[cfg(feature = "linux-pam-ext")]
             Exchange::RadioPrompt(p) => alloc(Style::RadioType, p.question()),
             #[cfg(feature = "linux-pam-ext")]
-            Exchange::BinaryPrompt(p) => Ok((Style::BinaryPrompt, unsafe {
-                CHeapBox::cast(CBinaryData::alloc(p.question())?)
-            })),
+            Exchange::BinaryPrompt(p) => {
+                let (data, typ) = p.question();
+                let payload = CHeapPayload::new(data, typ)?.into_inner();
+                Ok((Style::BinaryPrompt, unsafe { CHeapBox::cast(payload) }))
+            },
             #[cfg(not(feature = "linux-pam-ext"))]
             Exchange::RadioPrompt(_) | Exchange::BinaryPrompt(_) => {
                 Err(ErrorCode::ConversationError)
@@ -114,7 +118,7 @@
         }?;
         Ok(Self {
             style: style.into(),
-            data: Some(data),
+            data: Some(CHeapBox::into_ptr(data)),
         })
     }
 }
@@ -131,22 +135,22 @@
                     #[cfg(feature = "linux-pam-ext")]
                     Style::BinaryPrompt => self
                         .data
-                        .as_ref()
-                        .map(|p| CBinaryData::zero_contents(CHeapBox::as_ptr(p).cast())),
+                        .as_mut()
+                        .map(|p| BinaryPayload::zero(p.as_ptr().cast())),
                     #[cfg(feature = "linux-pam-ext")]
                     Style::RadioType => self
                         .data
-                        .as_ref()
-                        .map(|p| CHeapString::zero(CHeapBox::as_ptr(p).cast())),
+                        .as_mut()
+                        .map(|p| CHeapString::zero(p.cast())),
                     Style::TextInfo
                     | Style::ErrorMsg
                     | Style::PromptEchoOff
-                    | Style::PromptEchoOn => self
-                        .data
-                        .as_ref()
-                        .map(|p| CHeapString::zero(CHeapBox::as_ptr(p).cast())),
+                    | Style::PromptEchoOn => {
+                        self.data.as_mut().map(|p| CHeapString::zero(p.cast()))
+                    }
                 };
             };
+            let _ = self.data.map(|p| CHeapBox::from_ptr(p));
         }
     }
 }
@@ -178,6 +182,13 @@
     }
 }
 
+#[cfg(feature = "linux-pam-ext")]
+impl From<TooBigError> for ErrorCode {
+    fn from(_: TooBigError) -> Self {
+        ErrorCode::BufferError
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;