Re: libpam segfault when passwd passes NULL pamh (was Re: gcc -O2 produces invalid object code (x86_64, netbsd-5 branch))

[Richard Hansen <rhansen%bbn.com@localhost>]

Joerg Sonnenberger wrote:
> On Tue, Mar 09, 2010 at 01:05:19AM -0500, Richard Hansen wrote:
> > > Anything more than a plain number and fixed error message is not 
> > > possible with the constraints of the interface.
> >
> > I'm sorry, I'm still not following.  Are you arguing that 
> > pam_strerror() isn't needed for pam_start() failures because there's 
> > no useful information to convey to the user?
>
> No, just that the help it can give is very limited.

Ah, yes.  pam_strerror() could return a pointer to a static buffer that 
it fills with a rich error message, but that approach is problematic 
(concurrency, string lifetime, etc.).

> > > So Sun at least documents that pam_strerror(3) can be called 
> > > explicitly with NULL as handle if pam_start(3) failed.  XSSO is 
> > > silent on the topic, but it is the most sensible behavior.
> >
> > Agreed.
>
> I'll talk with DES about the issue, so that it can be adressed 
> upstream.

Cool, thanks!

In the meantime, it'd be nice if passwd was fixed (see attached patch). 
 Ideally the fix would be cherry picked to the netbsd-5 branch.  Should 
I submit a new problem report?  Or start a new thread on the appropriate 
mailing list?

Thanks,
Richard

Index: usr.bin/passwd/pam_passwd.c
===================================================================
RCS file: /NETBSD-CVS/src/usr.bin/passwd/pam_passwd.c,v
retrieving revision 1.5
diff -u -r1.5 pam_passwd.c
--- usr.bin/passwd/pam_passwd.c 2 Mar 2010 16:19:13 -0000       1.5
+++ usr.bin/passwd/pam_passwd.c 9 Mar 2010 18:41:41 -0000
@@ -73,6 +73,7 @@
 pwpam_process(const char *username, int argc, char **argv)
 {
        int ch, pam_err;
+       int pam_start_successful = 0;
        char hostname[MAXHOSTNAMELEN + 1];
 
        /* details about the invoking user for logging */
@@ -109,6 +110,7 @@
        /* initialize PAM -- always use the program name "passwd" */
        pam_err = pam_start("passwd", username, &pamc, &pamh);
        pam_check("unable to start PAM session");
+       pam_start_successful = 1;
 
        pam_err = pam_set_item(pamh, PAM_TTY, ttyname(STDERR_FILENO));
        pam_check("unable to set TTY");
@@ -140,7 +142,8 @@
        }
 
  end:
-       pam_end(pamh, pam_err);
+       if (pam_start_successful)
+           pam_end(pamh, pam_err);
        if (pam_err == PAM_SUCCESS)
                return;
        exit(1);