On Fri, Feb 08, 2013 at 01:00:55PM -0800, John Johansen wrote:

> @@ -1091,13 +1098,13 @@ ssize_t aa_replace_profiles(void *udata, size_t size, 
> bool noreplace)

Again, found while reviewing this patch, but not actually changed by
_this_ patch at all. (Sorry.) My comments are out here: vvvvvvvvvvvv

        /* released below */
        error = aa_unpack(udata, size, &lh, &ns_name);
        if (error)
                goto out;                               /* no ns yet */

        /* released below */
        ns = aa_prepare_namespace(ns_name);
        if (!ns) {
                info = "failed to prepare namespace";
                error = -ENOMEM;
                name = ns_name;
                goto fail;
        }

        /* ... */

out:
        aa_put_namespace(ns);                           /* but ns is put
                                                           and &lh is
                                                           not cleaned */

        if (error)
                return error;
        return size;

fail_lock:
        mutex_unlock(&ns->lock);

fail:
        error = audit_policy(op, GFP_KERNEL, name, info, error);

        list_for_each_entry_safe(new, tmp, &lh, base.list) {
                /* &lh list is private and not rcu based */
                list_del_init(&new->base.list);
                aa_free_profile(new);
        }

        goto out;
}



At least one case of aa_unpack (an error in a profile header) can cause
aa_unpack to leave profiles allocated on the list, so cleaning up after
it here makes sense. (Or having aa_unpack() tear down on failure. Either
way.)

Thanks

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to