* [PATCH] apparmor: Fix memleak issue in unpack_profile()
@ 2022-10-21 9:36 Xiu Jianfeng
2022-10-25 5:50 ` John Johansen
0 siblings, 1 reply; 2+ messages in thread
From: Xiu Jianfeng @ 2022-10-21 9:36 UTC (permalink / raw)
To: john.johansen, paul, serge; +Cc: apparmor, linux-security-module, linux-kernel
Before aa_alloc_profile(), it has allocated string for @*ns_name if @tmpns
is not NULL, so directly return -ENOMEM if aa_alloc_profile() failed will
cause a memleak issue, and even if aa_alloc_profile() succeed, in the
@fail_profile tag of aa_unpack(), it need to free @ns_name as well, this
patch fixes them.
Fixes: 736ec752d95e ("AppArmor: policy routines for loading and unpacking policy")
Fixes: 04dc715e24d0 ("apparmor: audit policy ns specified in policy load")
Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
---
security/apparmor/policy_unpack.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 2e028d540c6b..1bf8cfb8700a 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -858,8 +858,11 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
}
profile = aa_alloc_profile(name, NULL, GFP_KERNEL);
- if (!profile)
- return ERR_PTR(-ENOMEM);
+ if (!profile) {
+ info = "out of memory";
+ error = -ENOMEM;
+ goto fail;
+ }
rules = list_first_entry(&profile->rules, typeof(*rules), list);
/* profile renaming is optional */
@@ -1090,6 +1093,10 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
if (error == 0)
/* default error covers most cases */
error = -EPROTO;
+ if (*ns_name) {
+ kfree(*ns_name);
+ *ns_name = NULL;
+ }
if (profile)
name = NULL;
else if (!name)
@@ -1392,6 +1399,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
{
struct aa_load_ent *tmp, *ent;
struct aa_profile *profile = NULL;
+ char *ns_name = NULL;
int error;
struct aa_ext e = {
.start = udata->data,
@@ -1401,7 +1409,6 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
*ns = NULL;
while (e.pos < e.end) {
- char *ns_name = NULL;
void *start;
error = verify_header(&e, e.pos == e.start, ns);
if (error)
@@ -1432,6 +1439,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
ent->new = profile;
ent->ns_name = ns_name;
+ ns_name = NULL;
list_add_tail(&ent->list, lh);
}
udata->abi = e.version & K_ABI_MASK;
@@ -1452,6 +1460,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
return 0;
fail_profile:
+ kfree(ns_name);
aa_put_profile(profile);
fail:
--
2.17.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] apparmor: Fix memleak issue in unpack_profile()
2022-10-21 9:36 [PATCH] apparmor: Fix memleak issue in unpack_profile() Xiu Jianfeng
@ 2022-10-25 5:50 ` John Johansen
0 siblings, 0 replies; 2+ messages in thread
From: John Johansen @ 2022-10-25 5:50 UTC (permalink / raw)
To: Xiu Jianfeng, paul, serge; +Cc: apparmor, linux-security-module, linux-kernel
On 10/21/22 02:36, Xiu Jianfeng wrote:
> Before aa_alloc_profile(), it has allocated string for @*ns_name if @tmpns
> is not NULL, so directly return -ENOMEM if aa_alloc_profile() failed will
> cause a memleak issue, and even if aa_alloc_profile() succeed, in the
> @fail_profile tag of aa_unpack(), it need to free @ns_name as well, this
> patch fixes them.
>
> Fixes: 736ec752d95e ("AppArmor: policy routines for loading and unpacking policy")
> Fixes: 04dc715e24d0 ("apparmor: audit policy ns specified in policy load")
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
yep thanks I have pulled this into my tree
Acked-by: John Johansen <john.johansen@canonical.com>
> ---
> security/apparmor/policy_unpack.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index 2e028d540c6b..1bf8cfb8700a 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -858,8 +858,11 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
> }
>
> profile = aa_alloc_profile(name, NULL, GFP_KERNEL);
> - if (!profile)
> - return ERR_PTR(-ENOMEM);
> + if (!profile) {
> + info = "out of memory";
> + error = -ENOMEM;
> + goto fail;
> + }
> rules = list_first_entry(&profile->rules, typeof(*rules), list);
>
> /* profile renaming is optional */
> @@ -1090,6 +1093,10 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
> if (error == 0)
> /* default error covers most cases */
> error = -EPROTO;
> + if (*ns_name) {
> + kfree(*ns_name);
> + *ns_name = NULL;
> + }
> if (profile)
> name = NULL;
> else if (!name)
> @@ -1392,6 +1399,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
> {
> struct aa_load_ent *tmp, *ent;
> struct aa_profile *profile = NULL;
> + char *ns_name = NULL;
> int error;
> struct aa_ext e = {
> .start = udata->data,
> @@ -1401,7 +1409,6 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
>
> *ns = NULL;
> while (e.pos < e.end) {
> - char *ns_name = NULL;
> void *start;
> error = verify_header(&e, e.pos == e.start, ns);
> if (error)
> @@ -1432,6 +1439,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
>
> ent->new = profile;
> ent->ns_name = ns_name;
> + ns_name = NULL;
> list_add_tail(&ent->list, lh);
> }
> udata->abi = e.version & K_ABI_MASK;
> @@ -1452,6 +1460,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
> return 0;
>
> fail_profile:
> + kfree(ns_name);
> aa_put_profile(profile);
>
> fail:
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-10-25 5:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-21 9:36 [PATCH] apparmor: Fix memleak issue in unpack_profile() Xiu Jianfeng
2022-10-25 5:50 ` John Johansen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®