Messages in this thread | | | Date | Mon, 24 Oct 2022 22:50:33 -0700 | Subject | Re: [PATCH] apparmor: Fix memleak issue in unpack_profile() | From | John Johansen <> |
| |
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:
| |