login: Use getpwnam_r() instead of getpwnam().
ClosedPublic
Actions

Authored by des on Jan 9 2024, 6:09 PM.

Details

Reviewers

kevans
allanjude
imp
markj

Group Reviewers

Klara

Commits

rG9c738c4bca57: login: Use getpwnam_r() instead of getpwnam().
rGe3057ee0bf46: login: Use getpwnam_r() instead of getpwnam().
rGa3d80dd8aa6a: login: Use getpwnam_r() instead of getpwnam().

Summary

Since we expect the entry to still be valid after calling into PAM,
which may call getpwnam() itself, we need to use getpwnam_r().

MFC after: 1 week
Sponsored by: Klara, Inc.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

des created this revision.Jan 9 2024, 6:09 PM

Herald added a subscriber: imp. · View Herald TranscriptJan 9 2024, 6:09 PM

des requested review of this revision.Jan 9 2024, 6:09 PM

Harbormaster completed remote builds in B55331: Diff 132510.Jan 9 2024, 6:09 PM

kevans accepted this revision.Jan 9 2024, 6:37 PM

This revision is now accepted and ready to land.Jan 9 2024, 6:37 PM

Reviewed-by: allanjude

OK. Sure. But why? You're just swapping out one global for another

In D43376#989563, @imp wrote:

OK. Sure. But why? You're just swapping out one global for another

I believe the point is that the auth_pam() call can clobber the buffer.

In D43376#989563, @imp wrote:

OK. Sure. But why? You're just swapping out one global for another

We call pam_setcred() after calling getpwnam() and expect pwd to still be valid afterwards. PAM modules frequently also call getpwnam(). The only reason this hasn't gotten us into trouble yet is that they usually look up the same user, so even though pwd gets clobbered it still contains the correct data.