A couple of outstanding fixes to apparmor.
The following changes since commit b0af8dfdd67699e25083478c63eedef2e72ebd85:
Linux 3.0-rc5 (2011-06-27 19:12:22 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git for-security
John Johansen (2):
AppArmor: Fix reference to rcu protected pointer outside of rcu_read_lock
AppArmor: Fix masking of capabilities in complain mode
security/apparmor/domain.c | 2 +-
security/apparmor/lsm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
The pointer returned from tracehook_tracer_task() is only valid inside
the rcu_read_lock. However the tracer pointer obtained is being passed
to aa_may_ptrace outside of the rcu_read_lock critical section.
Mover the aa_may_ptrace test into the rcu_read_lock critical section, to
fix this.
Kernels affected: 2.6.36 - 3.0
Reported-by: Oleg Nesterov <[email protected]>
Signed-off-by: John Johansen <[email protected]>
---
security/apparmor/domain.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index c825c6e..78adc43 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -73,7 +73,6 @@ static int may_change_ptraced_domain(struct task_struct *task,
cred = get_task_cred(tracer);
tracerp = aa_cred_profile(cred);
}
- rcu_read_unlock();
/* not ptraced */
if (!tracer || unconfined(tracerp))
@@ -82,6 +81,7 @@ static int may_change_ptraced_domain(struct task_struct *task,
error = aa_may_ptrace(tracer, tracerp, to_profile, PTRACE_MODE_ATTACH);
out:
+ rcu_read_unlock();
if (cred)
put_cred(cred);
--
1.7.4.1
AppArmor is masking the capabilities returned by capget against the
capabilities mask in the profile. This is wrong, in complain mode the
profile has effectively all capabilities, as the profile restrictions are
not being enforced, merely tested against to determine is an access is
known by the profile.
This can result in the wrong behavior of security conscience applications
like sshd which examine their capability set, and change their behavior
accordingly. In this case because of the masked capability set being
returned sshd fails due to DAC checks, even when the profile is complain
mode.
Kernels affected: 2.6.36 - 3.0
Signed-off-by: John Johansen <[email protected]>
---
security/apparmor/lsm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 3d2fd14..3783202 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -127,7 +127,7 @@ static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
*inheritable = cred->cap_inheritable;
*permitted = cred->cap_permitted;
- if (!unconfined(profile)) {
+ if (!unconfined(profile) && !COMPLAIN_MODE(profile)) {
*effective = cap_intersect(*effective, profile->caps.allow);
*permitted = cap_intersect(*permitted, profile->caps.allow);
}
--
1.7.4.1
On Tue, Jun 28, 2011 at 04:56:05PM +0100, John Johansen wrote:
> AppArmor is masking the capabilities returned by capget against the
> capabilities mask in the profile. This is wrong, in complain mode the
> profile has effectively all capabilities, as the profile restrictions are
> not being enforced, merely tested against to determine is an access is
> known by the profile.
>
> This can result in the wrong behavior of security conscience applications
> like sshd which examine their capability set, and change their behavior
> accordingly. In this case because of the masked capability set being
> returned sshd fails due to DAC checks, even when the profile is complain
> mode.
>
> Kernels affected: 2.6.36 - 3.0
>
> Signed-off-by: John Johansen <[email protected]>
As this should probably go to the stable tree, next time care to add a
simple:
Cc: stable <[email protected]>
to the signed-off-by: area of the patch so it gets automagically
included in the stable and longterm kernels as needed when it hits
Linus's tree?
thanks,
greg k-h
On Tue, 28 Jun 2011, Greg KH wrote:
> As this should probably go to the stable tree, next time care to add a
> simple:
> Cc: stable <[email protected]>
> to the signed-off-by: area of the patch so it gets automagically
> included in the stable and longterm kernels as needed when it hits
> Linus's tree?
>
JJ, I'll wait for you to respin these before pulling them.
--
James Morris <[email protected]>