Skip to content

Fix/CI: Handle potential null, add Miri to CI #50

Merged
lvkv merged 2 commits into
mainfrom
lvkv/fixes-10
May 27, 2026
Merged

Fix/CI: Handle potential null, add Miri to CI #50
lvkv merged 2 commits into
mainfrom
lvkv/fixes-10

Conversation

@lvkv
Copy link
Copy Markdown
Owner

@lvkv lvkv commented May 27, 2026

The data argument supplied to the cleanup callback we hand to PAM can technically be null. This shouldn't normally happen (given the whole point of the cleanup callback), but the possibility needs to be handled regardless. This was actually caught by running miri on the test update included in this PR:

test module::tests::cleanup_disaster_scenarios ... error: Undefined Behavior: constructing invalid value: encountered 0, but expected something greater or equal to 1:
    |
90  |         unsafe { Unique { pointer: NonNull::new_unchecked(ptr), _marker: PhantomData } }
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0, but expected something greater or equal to 1
    |

Not unrelatedly, this PR also updates CI to add a miri test job.

@lvkv lvkv requested a review from Copilot May 27, 2026 18:36
@lvkv lvkv self-assigned this May 27, 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 cleanup callback against a possible null data pointer (preventing UB when constructing Box<T> from a null raw pointer) and adds a CI job to run Miri so similar issues are caught automatically.

Changes:

  • Add a defensive null check in cleanup<T> before Box::from_raw.
  • Extend/rename the unit test to cover “null data” and panic-on-drop disaster scenarios.
  • Add a GitHub Actions Miri job and adjust doc-test comment wording.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
pam/src/module.rs Adds null guarding in the PAM cleanup callback and updates tests to cover the null case.
.github/workflows/rust.yml Introduces a Miri CI job to detect UB in tests.

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

Comment thread .github/workflows/rust.yml
Comment thread .github/workflows/rust.yml
Comment thread .github/workflows/rust.yml
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 2 out of 2 changed files in this pull request and generated no new comments.

@lvkv lvkv merged commit a1dcc5b into main May 27, 2026
8 checks passed
@lvkv lvkv deleted the lvkv/fixes-10 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