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

["Markus Moeller" <huaraz@moeller.plus.com>]

Russ,

I think you need to set the whole structure to 0 after calloc.

memset(*creds, 0, sizeof(krb5_creds));

otherwise pointers like creds->client, creds->server could be undefined and 
then used in free calls later on in krb5_free_creds_contents (e.g. 
krb5_free_principal(context, creds->client);

Markus

"Russ Allbery" <rra@stanford.edu> wrote in message 
877igvo4d2.fsf@windlord.stanford.edu">news:877igvo4d2.fsf@windlord.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/>
>