[krbdev.mit.edu #1408] krb5_rc_dfl_expunge() bug

[Mike Eisler via RT]

This is in rc_dfl.c ... the bug is still in the 1.3 code I downloaded today.

krb5_error_code KRB5_CALLCONV
krb5_rc_dfl_expunge(krb5_context context, krb5_rcache id)
{
    struct dfl_data *t = (struct dfl_data *)id->data;
#ifdef NOIOSTUFF
    int i;
    struct authlist **q;
    struct authlist **qt;
    struct authlist *r;
    struct authlist *rt;

    for (q = &t->a; *q; q = qt) {
        qt = &(*q)->na;
        if (alive(context, &(*q)->rep, t->lifespan) == CMP_EXPIRED) {
            FREE((*q)->rep.client);
            FREE((*q)->rep.server);
            FREE(*q);
            *q = *qt; /* why doesn't this feel right? */
        }
    }


The comment "why doesn't this fell right?" is prophetic.

qt is &(*q)->qt, os *qt is refering to the (*q)->qt.
points to *q->na. However, *q was freed in the previous
statement. At that point, the memory allocator is free to
step on what *q pointed to, including *q->na. So in
some allocators (such as the one used by Network Appliance),
*q->na is now modified.

I think simply saving *qt into a temporary before
the FREE(*q) is the solution:

    r = *qt;
    FREE(*q);
    *q = r;

but the entire loop makes my brain hurt, and
I've chosen to re-write it using vairables named curr, prev, and
next, each of type authlist *.

BTW, I noticed that rep.magic is always 128, but I cannot
find where it gets set to 128.