Questions about iter check in profile_iterator()

[Will Fiveash]

On Mon, Jan 12, 2015 at 04:06:40PM -0800, Neng Xue wrote:
> Hi MIT krb team,
> 
> Recently I am working on a kerberos related project in Solaris which will
> upgrade our krb version to 1.13. We have a set of solaris specific profile
> interfaces underneath using MIT kerberos profile interfaces. They used to
> work fine. However, after the version upgrade I noticed the interface
> wrapper encountered an issue. I narrowed down the issue to:
> 
> *prof_get.c:profile_iterator()*:
> 
> 582 if (iter->magic != PROF_MAGIC_ITERATOR)
> 583        return PROF_MAGIC_ITERATOR;
> 
> When the iter is NULL our program will segmentation fault because of the
> null pointer deference.
> 
> Here is piece of our code:
> 
>       code = *profile_iterator_create*(profile, hierarchy,
> 42        PROFILE_ITER_LIST_SECTION, *&state*);
> 43    while (code == 0) {
> 44        code = *profile_iterator*(&*state*, &name, &value);
> 45        if (code == 0 && name != NULL) {
> 46
> 47            if (key != NULL && value != NULL) {
> 48                boolean_t ex_match = strcmp(key, value) ?
> 49                    B_FALSE : B_TRUE;
> 50                boolean_t match = strcasecmp(key, value) ?
> 51                    B_FALSE : B_TRUE;
> 52
> 53                if (ex_match == B_FALSE && case_ins == B_TRUE &&
> 54                    match == B_TRUE) {
> 55                    code2 = add_to_list(&values, name);
> 56                    if (code2 != 0) {
> 57                        end_list(&values, 0);
> 58                        code = code2;
> 59                    } else
> 60                        end_list(&values, &ret_values);
> 61                    goto cleanup;
> 62                } else if (ex_match == B_FALSE ||
> 63                    case_ins == B_TRUE)
> 64                    goto not_found;
> 65            }
> 66            code2 = add_to_list(&values, name);
> 67            if (code2 != 0) {
> 68                end_list(&values, 0);
> 69                code = code2;
> 70                goto cleanup;
> 71            }
> 72            found = B_TRUE;
> 73        }
> 74
> 75not_found:
> 76        if (name != NULL) {
> 77            profile_release_string(name);
> 78            name = NULL;
> 79        }
> 80        if (value != NULL) {
> 81            profile_release_string(value);
> 82            value = NULL;
> 83        }
> 84    }
> 
> The while loop will segmentation fault during the *third* loop. During the
> *second* loop, *profile_node_iterator()* underneath will free the iter
> pointer, then the third loop will still use that iter pointer to deference
> iter->magic value. So my question is that do we use the profile_iterator()
> in the wrong way or there should be an iter check in the profile_iterator()?
> Thanks.

Given what Neng has told me it looks like profile_iterator() is
returning 0 when iter->idata is NULL just after the call to
profile_node_iterator() which leads to (in profile_iterator):

    retval = profile_node_iterator(&iter->idata, 0, &name, &value);
    if (iter->idata == NULL) {  <----
        free(iter);  <----
        *iter_p = NULL;  <----
    }

Also there is the call to profile_node_iterator() and in the definition
of profile_node_iterator() I see:

errcode_t profile_node_iterator(void **iter_p, <--------
                                struct profile_node **ret_node,
                                char **ret_name, char **ret_value)

So in the profile_iterator call to profile_node_iterator() the
&iter->idata arg is iter_p for profile_node_iterator?  Seems odd to me.

-- 
Will Fiveash
Oracle Solaris Software Engineer