apparmor: access to uninitialized variable size may cause loop bounds overflow

[Colin Ian King]

Function unpack_secmark on a failed aa_unpack_array call may not set variable size and so the fail path is executing a loop using an undefined bounds on size.

VISIBLE_IF_KUNIT bool aa_unpack_array(struct aa_ext *e, const char *name, u16 *size)
{
        void *pos = e->pos;

        if (aa_unpack_nameX(e, AA_ARRAY, name)) {
                if (!aa_inbounds(e, sizeof(u16)))
                        goto fail;
                ^^ *size not set

                *size = le16_to_cpu(get_unaligned((__le16 *) e->pos));
                e->pos += sizeof(u16);
                return true;
        }

fail:
        e->pos = pos;
        return false;
}

....

static bool unpack_secmark(struct aa_ext *e, struct aa_ruleset *rules)
{
        void *pos = e->pos;
        u16 size;
        int i;

        if (aa_unpack_nameX(e, AA_STRUCT, "secmark")) {
                if (!aa_unpack_array(e, NULL, &size))
                        goto fail;

                ^^^ size is not set

                rules->secmark = kcalloc(size, sizeof(struct aa_secmark),
                                           GFP_KERNEL);
                if (!rules->secmark)
                        goto fail;

                rules->secmark_count = size;

                for (i = 0; i < size; i++) {
                        if (!unpack_u8(e, &rules->secmark[i].audit, NULL))
                                goto fail;
                        if (!unpack_u8(e, &rules->secmark[i].deny, NULL))
                                goto fail;
                        if (!aa_unpack_strdup(e, &rules->secmark[i].label, NULL))
                                goto fail;
                }
                if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
                        goto fail;
                if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
                        goto fail;
        }

        return true;

fail:
        if (rules->secmark) {
                for (i = 0; i < size; i++)
                        kfree(rules->secmark[i].label);

                ^^ for-loop on unbounded size

                kfree(rules->secmark);
                rules->secmark_count = 0;
                rules->secmark = NULL;
        }

        e->pos = pos;
        return false;
}