Re: Odd krb5_free_cred_contents problem (Heimdal 1.0.2, Solaris 9)

[Love Hörnquist Åstrand <lha@kth.se>]

Hello Russ,

I can't find why this would happen, care to run some more gdb ?

Love


23 feb 2008 kl. 14.32 skrev Russ Allbery:

> 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/ 
> >