2010-08-07 11:51:19

by Dan Carpenter

[permalink] [raw]
Subject: [patch] apparmor: issue with ns name without a following profile

If we have a ns name without a following profile then in the original
code it did "*ns_name = &name[1];". "name" is NULL so "*ns_name" is
0x1. That isn't useful and could cause an oops when this function is
called from aa_remove_profiles().

Signed-off-by: Dan Carpenter <[email protected]>
---
I'm not very familiar with this code and I haven't tested my fix.
Sorry. Please review carefully.

diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 6e85cdb..da34011 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -44,10 +44,12 @@ char *aa_split_fqname(char *fqname, char **ns_name)
/* overwrite ':' with \0 */
*split = 0;
name = skip_spaces(split + 1);
- } else
+ *ns_name = &name[1];
+ } else {
/* a ns name without a following profile is allowed */
+ *ns_name = &name[1];
name = NULL;
- *ns_name = &name[1];
+ }
}
if (name && *name == 0)
name = NULL;


2010-08-12 15:15:57

by John Johansen

[permalink] [raw]
Subject: Re: [patch] apparmor: issue with ns name without a following profile

On 08/07/2010 04:50 AM, Dan Carpenter wrote:
> If we have a ns name without a following profile then in the original
> code it did "*ns_name = &name[1];". "name" is NULL so "*ns_name" is
> 0x1. That isn't useful and could cause an oops when this function is
> called from aa_remove_profiles().
>
> Signed-off-by: Dan Carpenter <[email protected]>

Indeed. I am sorry to say this case was not enabled in the test suite :(
However proposed patch is incorrect, in that it results in namespace
name that starts at &name[1].

I've attached two patches, the first fixes this issue, and the second
fixes a locking bug in namespace removal, for this case (ie. where
there is no profile name specified.

Thanks for catching this

John


Attachments:
0001-AppArmor-Fix-splitting-an-fqname-into-separate-names.patch (1.61 kB)
0002-AppArmor-Fix-locking-from-removal-of-profile-namespa.patch (1.55 kB)
Download all attachments