Mercurial > crates > nonstick
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::*;