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
>
