aa_change_profile() invokes aa_get_current_label(), which returns
a reference of the current task's label.
According to the comment of aa_get_current_label(), the returned
reference must be put with aa_put_label().
However, when the original object pointed by "label" becomes
unreachable because aa_change_profile() returns or a new object
is assigned to "label", reference count increased by
aa_get_current_label() is not decreased, causing a refcnt leak.
Fix this by calling aa_put_label() before aa_change_profile() return
and dropping unnecessary aa_get_current_label().
Signed-off-by: Xiyu Yang <[email protected]>
Signed-off-by: Xin Tan <[email protected]>
---
Changes in v2:
- Remove unnecessary aa_get_current_label() because it gets called
earlier in the fn
---
security/apparmor/domain.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 6ceb74e0f789..a84ef030fbd7 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -1328,6 +1328,7 @@ int aa_change_profile(const char *fqname, int flags)
ctx->nnp = aa_get_label(label);
if (!fqname || !*fqname) {
+ aa_put_label(label);
AA_DEBUG("no profile name");
return -EINVAL;
}
@@ -1346,8 +1347,6 @@ int aa_change_profile(const char *fqname, int flags)
op = OP_CHANGE_PROFILE;
}
- label = aa_get_current_label();
-
if (*fqname == '&') {
stack = true;
/* don't have label_parse() do stacking */
--
2.7.4
> According to the comment of aa_get_current_label(), …
I suggest to make this wording clearer.
Would you like to refer to any software documentation here?
> However, when the original object pointed by "label" becomes
> unreachable because aa_change_profile() returns or a new object
> is assigned to "label", reference count increased by
> aa_get_current_label() is not decreased, causing a refcnt leak.
How do you think about to reduce abbreviations in the commit message?
Would you like to add the tag “Fixes” to the change description?
Regards,
Markus
On 4/15/20 4:27 AM, Markus Elfring wrote:
>> According to the comment of aa_get_current_label(), …
>
> I suggest to make this wording clearer.
> Would you like to refer to any software documentation here?
>
>
>> However, when the original object pointed by "label" becomes
>> unreachable because aa_change_profile() returns or a new object
>> is assigned to "label", reference count increased by
>> aa_get_current_label() is not decreased, causing a refcnt leak.
>
> How do you think about to reduce abbreviations in the commit message?
>
> Would you like to add the tag “Fixes” to the change description?
>
Fixes tags are always nice to have filled out, but some times its
hard to determine or the patch submitter doesn't know how or ...
If the fixes tags aren't there I will add them before I push them up.
In this case its
Fixes: 9fcf78cca198 ("apparmor: update domain transitions that are subsets of confinement at nnp")
> Regards,
> Markus
>