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 <[email protected]>

yep thanks I have pulled this into my tree

Acked-by: John Johansen <[email protected]>

---
  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:


Reply via email to