[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: libpam segfault when passwd passes NULL pamh (was Re: gcc -O2 produces invalid object code (x86_64, netbsd-5 branch))
On Mon, Mar 08, 2010 at 02:32:02PM -0500, Richard Hansen wrote:
> Joerg Sonnenberger wrote:
> >>My interpretation of pam_end(3) is that a NULL pamh is not
> >>prohibited, which would mean that passwd is not doing anything
> >I disagree. The handle is an abstract opaque type. An
> >implemenation that only ever allows a single PAM context being
> >open could just return NULL all the time for pam_start and it
> >would be within the spec.
> Good point. Let me restate to make sure I understand: For that
> hypothetical implementation, passing NULL to pam_end() would be
> appropriate because that's what pam_start() returns. Thus, apps
> should never test to see if pamh is NULL. Am I correct?
> >The openpam(3) implementation never returns NULL for pam_start and
> >therefore, NULL is not a valid handle to pass down to pam_end.
> So callers should only decide whether to call pam_end() or not based
> on pam_start()'s return code, correct? What do you think about the
> attached patch?
Not good enough for the issue you have outlined next.
> There's also the issue of whether it's OK to pass random garbage to
> pam_strerror(). If it's not OK, then how does one print error
> messages for pam_start() failures? pam_start() doesn't set pamh on
> error, so its value could be anything.
Fully agreed. Given that pam_start can only fail due to programming
errors or memory allocation, the error message is not very useful
anyway. I think the attached patch is the best approach.
RCS file: /home/joerg/repo/netbsd/src/usr.bin/passwd/pam_passwd.c,v
retrieving revision 1.5
diff -u -p -r1.5 pam_passwd.c
--- pam_passwd.c 2 Mar 2010 16:19:13 -0000 1.5
+++ pam_passwd.c 8 Mar 2010 19:38:44 -0000
@@ -108,7 +108,8 @@ pwpam_process(const char *username, int
/* initialize PAM -- always use the program name "passwd" */
pam_err = pam_start("passwd", username, &pamc, &pamh);
- pam_check("unable to start PAM session");
+ if (pam_err != PAM_SUCCESS)
+ errx(1, "unable to start PAM session");
pam_err = pam_set_item(pamh, PAM_TTY, ttyname(STDERR_FILENO));
pam_check("unable to set TTY");
Main Index |
Thread Index |