lkml.org 
[lkml]   [2019]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] apparmor: Fix use-after-free in aa_audit_rule_init
From
Date
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
>

\
 
 \ /
  Last update: 2019-10-20 20:51    [W:0.230 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site