Messages in this thread | | | Subject | Re: [PATCH] apparmor: Fix use-after-free in aa_audit_rule_init | From | John Johansen <> | Date | Sun, 20 Oct 2019 11:49:38 -0700 |
| |
On 10/20/19 7:16 AM, Markus Elfring wrote: >> … 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.
> > … >> +++ b/security/apparmor/audit.c > … >> @@ -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
>> aa_audit_rule_free(rule); >> - return PTR_ERR(rule->label); >> + return PTR_ERR(err); >> } >> >> *vrule = rule; > > > Regards, > Markus >
| |