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

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

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.

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

+++ 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

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

 	*vrule = rule;

Regards,
Markus