| Message ID | 1585493861-9867-1-git-send-email-xiyuyang19@fudan.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | apparmor: fix potential label refcnt leak in aa_change_profile | expand |
On 3/29/20 7:57 AM, Xiyu Yang wrote: > 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 the original object pointed > by "label" becomes unreachable. > > Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn> > Signed-off-by: Xin Tan <tanxin.ctf@gmail.com> > --- > security/apparmor/domain.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c > index 6ceb74e0f789..b99145ae34c0 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,6 +1347,7 @@ int aa_change_profile(const char *fqname, int flags) > op = OP_CHANGE_PROFILE; > } > > + aa_put_label(label); > label = aa_get_current_label(); this isn't quite right. Instead of adding the put_label here, the aa_get_current_label needs to be dropped, as it isn't needed because it gets called earlier in the fn. can you refresh with this change > > if (*fqname == '&') { >
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 6ceb74e0f789..b99145ae34c0 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,6 +1347,7 @@ int aa_change_profile(const char *fqname, int flags) op = OP_CHANGE_PROFILE; } + aa_put_label(label); label = aa_get_current_label(); if (*fqname == '&') {