Mercurial > crates > nonstick
changeset 101:94b51fa4f797
Fix memory soundness issues:
- Ensure Questions are pinned in memory when sending them through PAM.
- Hold on to the PAM conversation struct after we build it.
(Linux-PAM is leninent about this and copies the pam_conv structure.)
author | Paul Fisher <paul@pfish.zone> |
---|---|
date | Tue, 24 Jun 2025 17:54:33 -0400 |
parents | 3f11b8d30f63 |
children | 94eb11cb1798 |
files | src/libpam/answer.rs src/libpam/conversation.rs src/libpam/handle.rs src/libpam/pam_ffi.rs src/libpam/question.rs |
diffstat | 5 files changed, 32 insertions(+), 27 deletions(-) [+] |
line wrap: on
line diff
--- a/src/libpam/answer.rs Tue Jun 24 17:08:01 2025 -0400 +++ b/src/libpam/answer.rs Tue Jun 24 17:54:33 2025 -0400 @@ -6,10 +6,10 @@ pub use crate::libpam::pam_ffi::Answer; use crate::{ErrorCode, Result}; use std::ffi::CStr; +use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; use std::ptr::NonNull; -use std::{iter, mem, ptr, slice}; -use std::mem::ManuallyDrop; +use std::{iter, ptr, slice}; /// The corridor via which the answer to Messages navigate through PAM. #[derive(Debug)]
--- a/src/libpam/conversation.rs Tue Jun 24 17:08:01 2025 -0400 +++ b/src/libpam/conversation.rs Tue Jun 24 17:54:33 2025 -0400 @@ -2,7 +2,7 @@ use crate::conv::{Conversation, ErrorMsg, InfoMsg, MaskedQAndA, Message, QAndA}; use crate::libpam::answer::BinaryAnswer; use crate::libpam::answer::{Answer, Answers, TextAnswer}; -use crate::libpam::memory::{CBinaryData, Immovable}; +use crate::libpam::memory::CBinaryData; use crate::libpam::pam_ffi::AppData; pub use crate::libpam::pam_ffi::LibPamConversation; use crate::libpam::question::QuestionsTrait; @@ -21,7 +21,6 @@ callback: Self::wrapper_callback::<C>, appdata: (conv as *const C).cast(), life: PhantomData, - _marker: Immovable(PhantomData), } } @@ -66,13 +65,13 @@ impl Conversation for LibPamConversation<'_> { fn communicate(&self, messages: &[Message]) { let internal = || { - let questions = Questions::new(messages)?; + let questions = Box::pin(Questions::new(messages)?); let mut response_pointer = std::ptr::null_mut(); // SAFETY: We're calling into PAM with valid everything. let result = unsafe { (self.callback)( messages.len() as c_int, - questions.ptr(), + questions.as_ref().ptr(), &mut response_pointer, self.appdata, )
--- a/src/libpam/handle.rs Tue Jun 24 17:08:01 2025 -0400 +++ b/src/libpam/handle.rs Tue Jun 24 17:54:33 2025 -0400 @@ -11,7 +11,6 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::cell::Cell; use std::ffi::{c_char, c_int, CString}; -use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; use std::ptr; @@ -33,9 +32,12 @@ /// An owned PAM handle. pub struct OwnedLibPamHandle<'a> { + /// The handle itself. handle: HandleWrap, + /// The last return value from the handle. last_return: Cell<Result<()>>, - _conversation_lifetime: PhantomData<&'a mut ()>, + /// If set, the Conversation that this PAM handle owns. + conversation: Option<Box<LibPamConversation<'a>>>, } #[derive(Debug, PartialEq)] @@ -77,20 +79,26 @@ username: Option<String>, conversation: &impl Conversation, ) -> Result<Self> { - let conv = LibPamConversation::wrap(conversation); + let conv = Box::new(LibPamConversation::wrap(conversation)); let service_cstr = CString::new(service_name).map_err(|_| ErrorCode::ConversationError)?; let username_cstr = memory::prompt_ptr(memory::option_cstr(username.as_deref())?.as_ref()); let mut handle: *mut LibPamHandle = ptr::null_mut(); // SAFETY: We've set everything up properly to call `pam_start`. // The returned value will be a valid pointer provided the result is OK. - let result = - unsafe { pam_ffi::pam_start(service_cstr.as_ptr(), username_cstr, &conv, &mut handle) }; + let result = unsafe { + pam_ffi::pam_start( + service_cstr.as_ptr(), + username_cstr, + conv.as_ref(), + &mut handle, + ) + }; ErrorCode::result_from(result)?; Ok(Self { handle: HandleWrap(handle), last_return: Cell::new(Ok(())), - _conversation_lifetime: Default::default(), + conversation: Some(conv), }) } }
--- a/src/libpam/pam_ffi.rs Tue Jun 24 17:08:01 2025 -0400 +++ b/src/libpam/pam_ffi.rs Tue Jun 24 17:54:33 2025 -0400 @@ -57,7 +57,6 @@ /// but for requests with style `PAM_BINARY_PROMPT`, /// this will be `CBinaryData` (a Linux-PAM extension). pub data: Option<CHeapBox<c_void>>, - pub _marker: Immovable, } /// The callback that PAM uses to get information in a conversation. @@ -86,8 +85,9 @@ /// The pointer that will be passed as the last parameter /// to the conversation callback. pub appdata: *const AppData, + /// Marker to associate the lifetime of this with the conversation + /// that was passed in. pub life: PhantomData<&'a mut ()>, - pub _marker: Immovable, } /// Gets a string version of an error message.
--- a/src/libpam/question.rs Tue Jun 24 17:08:01 2025 -0400 +++ b/src/libpam/question.rs Tue Jun 24 17:54:33 2025 -0400 @@ -11,6 +11,7 @@ use crate::Result; use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::ffi::{c_void, CStr}; +use std::pin::Pin; use std::slice; /// Abstraction of a collection of questions to be sent in a PAM conversation. @@ -64,7 +65,7 @@ Self: Sized; /// Gets the pointer that is passed . - fn ptr(&self) -> *const *const Question; + fn ptr(self: Pin<&Self>) -> *const *const Question; /// Converts a pointer into a borrowed list of Questions. /// @@ -115,7 +116,7 @@ }) } - fn ptr(&self) -> *const *const Question { + fn ptr(self: Pin<&Self>) -> *const *const Question { &self.pointer as *const *const Question } @@ -131,10 +132,9 @@ #[derive(Debug)] #[repr(C)] pub struct LinuxPamQuestions { - #[allow(clippy::vec_box)] // we need to do this. + #[allow(clippy::vec_box)] // we need to box vec items. /// The place where the questions are. questions: Vec<Box<Question>>, - _marker: Immovable, } impl LinuxPamQuestions { @@ -155,11 +155,10 @@ .collect(); Ok(Self { questions: questions?, - _marker: Default::default(), }) } - fn ptr(&self) -> *const *const Question { + fn ptr(self: Pin<&Self>) -> *const *const Question { self.questions.as_ptr().cast() } @@ -246,7 +245,6 @@ Ok(Self { style: style.into(), data: Some(data), - _marker: Default::default(), }) } } @@ -328,15 +326,15 @@ use super::super::*; #[test] fn standard() { - let interrogation = <$typ>::new(&[ + let interrogation = Box::pin(<$typ>::new(&[ MaskedQAndA::new("hocus pocus").message(), QAndA::new("what").message(), QAndA::new("who").message(), InfoMsg::new("hey").message(), ErrorMsg::new("gasp").message(), ]) - .unwrap(); - let indirect = interrogation.ptr(); + .unwrap()); + let indirect = interrogation.as_ref().ptr(); let remade = unsafe { $typ::borrow_ptr(indirect, interrogation.len()) }; let messages: Vec<OwnedMessage> = remade @@ -364,11 +362,11 @@ #[test] #[cfg(feature = "linux-pam-extensions")] fn linux_extensions() { - let interrogation = <$typ>::new(&[ + let interrogation = Box::pin(<$typ>::new(&[ BinaryQAndA::new((&[5, 4, 3, 2, 1], 66)).message(), RadioQAndA::new("you must choose").message(), - ]).unwrap(); - let indirect = interrogation.ptr(); + ]).unwrap()); + let indirect = interrogation.as_ref().ptr(); let remade = unsafe { $typ::borrow_ptr(indirect, interrogation.len()) }; let messages: Vec<OwnedMessage> = remade