Skip to content

Lack of proper linking during the build process - #546

Merged
bilelmoussaoui merged 2 commits into
linux-credentials:mainfrom
justpav05:main
Aug 15, 2026
Merged

Lack of proper linking during the build process#546
bilelmoussaoui merged 2 commits into
linux-credentials:mainfrom
justpav05:main

Conversation

@justpav05

Copy link
Copy Markdown
Contributor

The PAM module didn't specify #[link(name = "pam")] in the extern "C" block, so the linker never emitted -lpam. PAM then failed to dlopen the module with undefined symbol errors for pam_get_item, pam_get_user, pam_set_data, and pam_get_data.

Please either add this yourself or accept a PR to prevent this from happening again and ensure PAM authentication works properly

The PAM module didn't specify #[link(name = "pam")] on the
extern "C" block, so the linker never emitted -lpam. PAM then
failed to dlopen the module with undefined symbol errors for
pam_get_item, pam_get_user, pam_set_data, pam_get_data
@A6GibKm

A6GibKm commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

See gtk-rs/gtk-rs-core@8e0a85a, this shouldn't be necessary. Note that pam authentication IS working on GNOME OS.

@justpav05

Copy link
Copy Markdown
Contributor Author

Are you saying this will work ONLY for GNOME OS?

And what does that commit have to do with it? It’s completely irrelevant?

@A6GibKm

A6GibKm commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Are you saying this will work ONLY for GNOME OS?

No, I am saying that we know that it work at least on GNOME OS. It has also been tested on fedora.

And what does that commit have to do with it? It’s completely irrelevant?

It removed the link=name (which can also be defined via cargo.toml) from all the rust bindings since in that case it was deemed it was not doing anything.

@justpav05

Copy link
Copy Markdown
Contributor Author

Yes, but it doesn't work on CachyOS. The problem is that if you add that line, it links correctly with libpam, and then everything starts working

@justpav05

Copy link
Copy Markdown
Contributor Author

If necessary, I’m ready to add a commit to centralize it in Cargo.toml.

@bilelmoussaoui

Copy link
Copy Markdown
Collaborator

If necessary, I’m ready to add a commit to centralize it in Cargo.toml.

Can you test if it helps in your case?

@justpav05

Copy link
Copy Markdown
Contributor Author

I’ve already built and tested the PR and am using it right now.

I don't quite understand the part about Cargo.

Do you want me to add:

[target.x86_64-unknown-linux-gnu]
rustflags = ["-l", "pam"]

@justpav05

Copy link
Copy Markdown
Contributor Author

Is this already closed?

@A6GibKm

A6GibKm commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

See the commit I linked above for an example on how to set it on Cargo.toml.

@justpav05

Copy link
Copy Markdown
Contributor Author

U mean this commit - gtk-rs/gtk-rs-core@8e0a85a?

@justpav05

Copy link
Copy Markdown
Contributor Author

And silent...

@justpav05

Copy link
Copy Markdown
Contributor Author

So?

@bilelmoussaoui

Copy link
Copy Markdown
Collaborator

No, the suggestion is to use links in Cargo.toml and see if that helps.

@justpav05

Copy link
Copy Markdown
Contributor Author

links = "pam" in Cargo.toml won't do what you're expecting here. It isn't a linking directive — it only declares that the crate owns a native library (so Cargo can enforce that at most one crate in the graph links it) and enables DEP_* metadata passing between build scripts. It emits no -l flags by itself, and Cargo rejects it outright without a build script: package specifies that it links to 'pam' but does not have a custom build script.

But the oo7 has no build.rs.

The commit you linked is about exactly that manifest key — it removes links = "cairo", links = "gio-2.0" etc. from the -sys crates. It does not touch #[link(name = ...)] attributes; in gtk-rs the actual linking is done by build scripts via system-deps, which emit cargo:rustc-link-lib=.

So it isn't evidence that the attribute is redundant here — if anything it argues against adding links to oo7-pam.

With no link directive of any kind, pam_oo7.so gets no DT_NEEDED entry for libpam.so.0:

$ ldd libpam_oo7.so | grep -i pam
<paste>
$ nm -D --undefined-only libpam_oo7.so | grep pam_
<paste>

That happens to work whenever the process calling dlopen() already has libpam in its global symbol scope, which covers most login paths — hence it being fine on GNOME OS and Fedora. Here it fails with:

<paste journalctl error>

If you'd prefer to keep the directive out of the source, I'm happy to switch the PR to a build script instead — functionally identical to the attribute:

// pam/build.rs
fn main() {
    println!("cargo::rustc-link-lib=pam");
}

Happy to go with whichever you prefer.

@justpav05

Copy link
Copy Markdown
Contributor Author

The problem is that pam wasn't added as a mandatory build dependency in the CI Action. What are you running it on? Ubuntu?

@bilelmoussaoui

Copy link
Copy Markdown
Collaborator

you will need to add it to both https://github.com/linux-credentials/oo7/blob/main/.github/workflows/CI.yml#L127 (fedora) and https://github.com/linux-credentials/oo7/blob/main/.github/workflows/CI.yml#L57 (ubuntu). The problem i see with this is that our docs.rs job will fail now given that pam, the C library won't be found on docs.rs. So we need to do the same trick as it is done in gtk-rs with https://github.com/gtk-rs/gtk-rs-core/blob/main/glib/sys/build.rs#L5-L15

so we should do make use of build.rs instead of using the link attribute

@bilelmoussaoui
bilelmoussaoui merged commit c2aa231 into linux-credentials:main Aug 15, 2026
8 checks passed
@bilelmoussaoui

Copy link
Copy Markdown
Collaborator

Thanks!

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.

3 participants