2011-05-31 18:31:44

by Kees Cook

[permalink] [raw]
Subject: [PATCH] AppArmor: fix oops in apparmor_setprocattr

When invalid parameters are passed to apparmor_setprocattr a NULL deref
oops occurs when it tries to record an audit message. This is because
it is passing NULL for the profile parameter for aa_audit. But aa_audit
now requires that the profile passed is not NULL.

Fix this by passing the current profile on the task that is trying to
setprocattr.

Signed-off-by: Kees Cook <[email protected]>
Signed-off-by: John Johansen <[email protected]>
---
security/apparmor/lsm.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index ae3a698..ec1bcec 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -593,7 +593,8 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
sa.aad.op = OP_SETPROCATTR;
sa.aad.info = name;
sa.aad.error = -EINVAL;
- return aa_audit(AUDIT_APPARMOR_DENIED, NULL, GFP_KERNEL,
+ return aa_audit(AUDIT_APPARMOR_DENIED,
+ __aa_current_profile(), GFP_KERNEL,
&sa, NULL);
}
} else if (strcmp(name, "exec") == 0) {
--
1.7.4.1


--
Kees Cook
Ubuntu Security Team


2011-06-01 00:25:40

by James Morris

[permalink] [raw]
Subject: Re: [PATCH] AppArmor: fix oops in apparmor_setprocattr

On Tue, 31 May 2011, Kees Cook wrote:

> When invalid parameters are passed to apparmor_setprocattr a NULL deref
> oops occurs when it tries to record an audit message. This is because
> it is passing NULL for the profile parameter for aa_audit. But aa_audit
> now requires that the profile passed is not NULL.
>
> Fix this by passing the current profile on the task that is trying to
> setprocattr.

Is this trigger able by unprivileged users?

>From which upstream commit is this an issue?


- James
--
James Morris
<[email protected]>

2011-06-01 00:46:20

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH] AppArmor: fix oops in apparmor_setprocattr

James Morris wrote:
> Is this trigger able by unprivileged users?

Anybody who can call .setprocattr LSM hook can trigger this bug, but for most
configurations BUG_ON() will prevent NULL pointer dereference.

179 int aa_audit(int type, struct aa_profile *profile, gfp_t gfp,
180 struct common_audit_data *sa,
181 void (*cb) (struct audit_buffer *, void *))
182 {
183 BUG_ON(!profile);

> From which upstream commit is this an issue?

Since 2.6.36.

2011-06-01 02:02:59

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] AppArmor: fix oops in apparmor_setprocattr

Hi James,

On Wed, Jun 01, 2011 at 10:25:37AM +1000, James Morris wrote:
> On Tue, 31 May 2011, Kees Cook wrote:
> > When invalid parameters are passed to apparmor_setprocattr a NULL deref
> > oops occurs when it tries to record an audit message. This is because
> > it is passing NULL for the profile parameter for aa_audit. But aa_audit
> > now requires that the profile passed is not NULL.
> >
> > Fix this by passing the current profile on the task that is trying to
> > setprocattr.
>
> Is this trigger able by unprivileged users?

Yes, but the Oops kills the process doing the write as it triggers
the BUG_ON, not the process being written to.

> >From which upstream commit is this an issue?

b5e95b48685e3481139a5634d14d630d12c7d5ce

-Kees

--
Kees Cook
Ubuntu Security Team