Odd krb5_free_cred_contents problem (Heimdal 1.0.2, Solaris 9)

[Russ Allbery <rra@stanford.edu>]

Alf ran into an odd problem with my pam-krb5 module built statically with
Heimdal 1.0.2 on Solaris 9 with the Sun Studio C version 11 compiler.

pam-krb5, when authentication failed, was dying with:

23484:      Incurred fault #5, FLTACCESS  %pc = 0x001E1668
23484:        siginfo: SIGBUS BUS_ADRALN addr=0xAACA6001
23484:      Received signal #10, SIGBUS [default]
23484:        siginfo: SIGBUS BUS_ADRALN addr=0xAACA6001

Inside the module, it does:

int
pamk5_password_auth(struct pam_args *args, const char *service,
                    krb5_creds **creds)
{
/* ... */
    *creds = calloc(1, sizeof(krb5_creds));
/* ... */
            retval = krb5_get_init_creds_password(ctx->context, *creds,
                          ctx->princ, pass, pamk5_prompter_krb5, args, 0,
                          (char *) service, opts);
            success = (retval == 0) ? PAM_SUCCESS : PAM_AUTH_ERR;
/* ... */
        if (*creds != NULL) {
            krb5_free_cred_contents(ctx->context, *creds);
            free(*creds);
            *creds = NULL;
        }

The krb5_free_cred_contents here appears to be the culprit, even though
the structure is initialized all-zeros.  If I apply this patch, the crash
goes away:

--- auth.c      2007-12-26 04:19:34 +0000
+++ auth.c      2008-02-20 22:41:06 +0000
@@ -485,7 +485,7 @@
 {
     struct context *ctx;
     krb5_get_init_creds_opt *opts = NULL;
-    int retval, retry, success;
+    int retval, retry, success, creds_valid;
     char *pass = NULL;
     int authtok = (service == NULL) ? PAM_AUTHTOK : PAM_OLDAUTHTOK;
 
@@ -543,6 +543,7 @@
         pamk5_error(args, "cannot allocate memory: %s", strerror(errno));
         return PAM_SERVICE_ERR;
     }
+    creds_valid = 0;
     retval = pamk5_compat_opt_alloc(ctx->context, &opts);
     if (retval != 0) {
         pamk5_error_krb5(args, "cannot allocate credential options", retval);
@@ -612,6 +613,8 @@
     } while (retry && retval == KRB5KRB_AP_ERR_BAD_INTEGRITY);
     if (retval != 0)
         pamk5_debug_krb5(args, "krb5_get_init_creds_password", retval);
+    else
+        creds_valid = 1;
 
 done:
     /*
@@ -632,7 +635,8 @@
         retval = PAM_SUCCESS;
     else {
         if (*creds != NULL) {
-            krb5_free_cred_contents(ctx->context, *creds);
+            if (creds_valid)
+                krb5_free_cred_contents(ctx->context, *creds);
             free(*creds);
             *creds = NULL;
         }

However, looking at the source code, it looks like all of the frees are
guarded by checks for whether the pointers are NULL, so I don't understand
why this would be necessary.  (I originally didn't do things this way both
because it was simpler the other way and because I wasn't positive that 
krb5_get_init_creds_password would never allocate memory in *creds that
had to be freed even on a failed authentication.)

Is the above patch the right fix, or is there something else going on?

-- 
Russ Allbery (rra@stanford.edu)             <http://www.eyrie.org/~eagle/>