Skip to content

Fix: FFI return code handling#46

Merged
lvkv merged 3 commits into
mainfrom
lvkv/fixes-5
May 25, 2026
Merged

Fix: FFI return code handling#46
lvkv merged 3 commits into
mainfrom
lvkv/fixes-5

Conversation

@lvkv
Copy link
Copy Markdown
Owner

@lvkv lvkv commented May 25, 2026

The PAM API returns a c_int code for most functions. The current FFI bindings interpret this as a PamResultCode enum that's #[repr(C)], but only defines enum values from 0 to 31 inclusive. Any code returned outside of this range is UB (e.g., https://users.rust-lang.org/t/guarding-against-invalid-enum-values-arriving-via-ffi/33482). This fixes that. Invalid codes are squashed into a PAM_SYSTEM_ERR where appropriate.

This also updates set_data to:

  • Avoid a potential leak if pam_set_data returns an error
  • Take &mut self on the PAM handle to prevent being called while get_data references still exist. I.e., it mutates, so require mut.
  • Require 'static on the T being stored to ensure that the T doesn't secretly borrow anything that might be dropped while PAM still holds it

@lvkv lvkv requested a review from Copilot May 25, 2026 20:00
@lvkv lvkv self-assigned this May 25, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the PAM Rust bindings by preventing UB when PAM (or the conversation callback) returns a c_int that doesn’t correspond to a defined PamResultCode enum discriminant.

Changes:

  • Update PAM FFI function bindings to return c_int instead of PamResultCode.
  • Add TryFrom<c_int> and an internal PamResultCode::from_raw(c_int) helper to safely convert raw return codes (mapping unknowns to PAM_SYSTEM_ERR).
  • Apply safe conversion at call sites in module.rs and conv.rs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
pam/src/module.rs Switch PAM FFI return types to c_int and convert via PamResultCode::from_raw at call sites.
pam/src/conv.rs Update conversation callback return type to c_int and convert via PamResultCode::from_raw.
pam/src/constants.rs Implement safe conversion from raw c_int to PamResultCode (including handling unknown values).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pam/src/module.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@lvkv lvkv merged commit b0c86a0 into main May 25, 2026
6 checks passed
@lvkv lvkv deleted the lvkv/fixes-5 branch May 27, 2026 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants