On Sun, Oct 20, 2019 at 1:51 PM John Johansen
[off-list ref] wrote:

On 10/20/19 7:16 AM, Markus Elfring wrote:

quoted

quoted

… But after this release the the return statement
tries to access the label field of the rule which results in
use-after-free. Before releaseing the rule, copy errNo and return it
after releasing rule.

Navid thanks for finding this, and Markus thanks for the review

quoted

Please avoid a duplicate word and a typo in this change description.
My preference would be a v2 version of the patch with the small clean-ups

that Markus has pointed out.

John and Markus, I updated and submitted v2.

If I don't see a v2 this week I can pull this one in and do the revisions
myself adding a little fix-up note.

quoted

…

quoted

+++ b/security/apparmor/audit.c

…

quoted

@@ -197,8 +198,9 @@ int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
     rule->label = aa_label_parse(&root_ns->unconfined->label, rulestr,
                                  GFP_KERNEL, true, false);
     if (IS_ERR(rule->label)) {
+            err = rule->label;

How do you think about to define the added local variable in this if branch directly?

+             int err = rule->label;

yes, since err isn't defined or in use else where this would be preferable

quoted

quoted

             aa_audit_rule_free(rule);
-            return PTR_ERR(rule->label);
+            return PTR_ERR(err);
     }

     *vrule = rule;

Regards,
Markus

-- 
Thanks,
Navid.