Skip to content

src/openrc-user/openrc-user.c: Preserve struct passwd from PAM#984

Merged
navi-desu merged 1 commit intoOpenRC:masterfrom
salahcoronya:openrc-user-getpwnam_r
Mar 2, 2026
Merged

src/openrc-user/openrc-user.c: Preserve struct passwd from PAM#984
navi-desu merged 1 commit intoOpenRC:masterfrom
salahcoronya:openrc-user-getpwnam_r

Conversation

@salahcoronya
Copy link
Contributor

@salahcoronya salahcoronya commented Mar 1, 2026

Some PAM modules call getpwnam(), so pam_open_session can clobber our copy. Use getpwnam_r instead.

Closes: #979

@salahcoronya salahcoronya force-pushed the openrc-user-getpwnam_r branch 2 times, most recently from 6b0cd04 to fb2a25e Compare March 1, 2026 20:52
@navi-desu
Copy link
Member

a) in truth, this is a sssd bug (and a bad one), no library should be using global storage, we have to report it to them

b) this does not handle ERANGE, opting to abort on any error instead

c) after initializing the environment, we only need uid, gid, and pw_shell -- the former two are trivially copiable, so, like i mentioned on the gentoo bug, we want to xstrdup pw_shell and pass that directly instead

@salahcoronya salahcoronya force-pushed the openrc-user-getpwnam_r branch from fb2a25e to 134c2b3 Compare March 1, 2026 22:59
@salahcoronya
Copy link
Contributor Author

Ok, I respun the patch to use the copy/xstrdup approach instead. Or am I barking up the wrong tree here, and is really a bug with sssd?

@salahcoronya salahcoronya changed the title src/openrc-user/openrc-user.c: Preserver struct passwd from PAM src/openrc-user/openrc-user.c: Preserve struct passwd from PAM Mar 2, 2026
salahcoronya added a commit to salahcoronya/sssd that referenced this pull request Mar 2, 2026
…sswd

If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls
something like pam_open_session, sssd's call to getpwnam in
init_sssd_ids clobbers the cached value by the other program.

Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
salahcoronya added a commit to salahcoronya/sssd that referenced this pull request Mar 2, 2026
…sswd

If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls
something like pam_open_session, sssd's call to getpwnam in
init_sssd_ids clobbers the cached value by the other program.

Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
salahcoronya added a commit to salahcoronya/sssd that referenced this pull request Mar 2, 2026
…sswd

If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls
something like pam_open_session, sssd's call to getpwnam in
init_sssd_ids clobbers the cached value by the other program.

Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
salahcoronya added a commit to salahcoronya/sssd that referenced this pull request Mar 2, 2026
…sswd

If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls
something like pam_open_session, sssd's call to getpwnam in
init_sssd_ids clobbers the cached value by the other program.

Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
parts we need */
pwd.pw_uid = user->pw_uid;
pwd.pw_gid = user->pw_gid;
pwd.pw_shell = xstrdup(user->pw_shell);
Copy link
Contributor

@N-R-K N-R-K Mar 2, 2026

Choose a reason for hiding this comment

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

I would store them in different variables instead and change spawn_openrc to accept them separately, instead of a dummy passwd struct.

Having a struct only partially initialized like this is planting a footgun.

Copy link
Member

Choose a reason for hiding this comment

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

i'm just going to split that when merging this, because agree

@navi-desu
Copy link
Member

Ok, I respun the patch to use the copy/xstrdup approach instead. Or am I barking up the wrong tree here, and is really a bug with sssd?

it is a bug with sssd -- i'll merge this as a workaround but sssd really should not be using getpwnam in a dynamically loaded module, that's dangerous and we really should open a bug with them

@navi-desu navi-desu force-pushed the openrc-user-getpwnam_r branch from 134c2b3 to 37ed847 Compare March 2, 2026 11:03
sssd's pam module seems to call getpwnam, clobbering the global struct.
while this is a bug on sssd, let's copy the data out of it as a
workaround

Closes: github.com/OpenRC/issues/979
Bug: https://bugs.gentoo.org/970235
Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
@navi-desu navi-desu force-pushed the openrc-user-getpwnam_r branch from 37ed847 to cf3eef0 Compare March 2, 2026 11:04
@navi-desu navi-desu merged commit e638d4b into OpenRC:master Mar 2, 2026
7 checks passed
salahcoronya added a commit to salahcoronya/sssd that referenced this pull request Mar 2, 2026
…sswd

If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls
something like pam_open_session, sssd's call to getpwnam in
init_sssd_ids clobbers the cached value by the other program.

Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
salahcoronya added a commit to salahcoronya/sssd that referenced this pull request Mar 2, 2026
…sswd

If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls
something like pam_open_session, sssd's call to getpwnam in
init_sssd_ids clobbers the cached value by the other program.

Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
salahcoronya added a commit to salahcoronya/sssd that referenced this pull request Mar 2, 2026
…sswd

If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls
something like pam_open_session, sssd's call to getpwnam in
init_sssd_ids clobbers the cached value by the other program.

Signed-off-by: Christopher Byrne <salah.coronya@gmail.com>
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.

openrc-user: buffer returned by getpwname() may not survive pam_open_session

3 participants