fix: pam_set_data error handling, setsid+fd sweep in grandchild, is_replace logic, null pamh guard

crates/ahfail-pam/src/lib.rs

@@ -10,7 +10,6 @@ pub const PAM_IGNORE: c_int = 25;
 pub const PAM_AUTH_ERR: c_int = 7;
 #[cfg(target_os = "macos")]
 pub const PAM_AUTH_ERR: c_int = 9;
-// Fallback for CI on non-Linux non-macOS (should not occur in production)
 #[cfg(not(any(target_os = "linux", target_os = "macos")))]
 pub const PAM_AUTH_ERR: c_int = 7;
 
@@ -55,7 +54,7 @@ unsafe extern "C" fn ahfail_cleanup(
     data: *mut c_void,
     error_status: c_int,
 ) {
-    // Reclaim any stored path string to avoid a leak.
+    // Reclaim any stored path string to avoid a leak (PAM guarantees this fires exactly once).
     let path_override: Option<String> = if data.is_null() {
         None
     } else {
@@ -63,8 +62,11 @@ unsafe extern "C" fn ahfail_cleanup(
     };
 
     if is_replace(error_status) {
-        // Previous attempt failed, same PAM handle; a new attempt is starting.
-        spawn_display(path_override);
+        // Data replaced — a new pam_set_data call overwrote ours. Only spawn on failure;
+        // on success (PAM_SUCCESS | PAM_DATA_REPLACE) we do nothing.
+        if is_failure(error_status) {
+            spawn_display(path_override);
+        }
         return;
     }
     if is_failure(error_status) {
@@ -83,10 +85,12 @@ pub unsafe extern "C" fn pam_sm_authenticate(
     argc: c_int,
     argv: *const *const c_char,
 ) -> c_int {
+    if pamh.is_null() { return PAM_IGNORE; }
+
     let args = argc_argv(argc, argv);
     let display_path = read_display_path_arg(args);
 
-    let key = match CString::new("ahfail") {
+    let key = match CString::new("dk.weircon.ahfail") {
         Ok(k) => k,
         Err(_) => return PAM_IGNORE,
     };
@@ -97,7 +101,11 @@ pub unsafe extern "C" fn pam_sm_authenticate(
         None => std::ptr::null_mut(),
     };
 
-    pam_set_data(pamh, key.as_ptr(), path_ptr, Some(ahfail_cleanup));
+    let ret = pam_set_data(pamh, key.as_ptr(), path_ptr, Some(ahfail_cleanup));
+    if ret != PAM_SUCCESS && !path_ptr.is_null() {
+        // pam_set_data failed — cleanup will never fire, so free the Box ourselves.
+        drop(Box::from_raw(path_ptr as *mut String));
+    }
     PAM_IGNORE
 }
 
@@ -163,8 +171,12 @@ fn spawn_display(path_override: Option<String>) {
         // Intermediate: fork again then exit immediately.
         let pid2: pid_t = libc::fork();
         if pid2 != 0 { libc::_exit(0); }
-        // Grandchild: close inherited fds, exec ahfail-display.
-        libc::close(0); libc::close(1); libc::close(2);
+        // Grandchild: detach from PAM daemon's session, close all inherited fds, exec.
+        libc::setsid();
+        let max_fd = libc::sysconf(libc::_SC_OPEN_MAX) as c_int;
+        for fd in 0..max_fd.min(4096) {
+            libc::close(fd);
+        }
         let args: [*const c_char; 2] = [cpath.as_ptr(), std::ptr::null()];
         libc::execv(cpath.as_ptr(), args.as_ptr());
         libc::_exit(1);