diff mbox

[3/5] UBUNTU: SAUCE: AppArmor: Fix cap audit_caching preemption disabling

Message ID 1257877753-9448-4-git-send-email-john.johansen@canonical.com
State Accepted
Headers show

Commit Message

John Johansen Nov. 10, 2009, 6:29 p.m. UTC
BugLink: http://bugs.launchpad.net/bugs/479102

SRU Justification: Failing to put_cpu_var means that kernel preemption is
disabled for the task. This will affect all confined processes that try
to audit a capability message (so an process that has capability violation
or is in learning mode and would have a capability violation).

The auditing code of capabilities, has a simple cache to reduce capability
messages flooding the audit logs.  Checking and updating the cache
disables kernel preemption.  One potential exit path does not properly
put the per cpu var, thus not reenabling preemption.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 ubuntu/apparmor/capability.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Comments

Stefan Bader Nov. 11, 2009, 1:21 p.m. UTC | #1
Clearly fixes the described problem as it matches the get with a put.

John Johansen wrote:
> BugLink: http://bugs.launchpad.net/bugs/479102
> 
> SRU Justification: Failing to put_cpu_var means that kernel preemption is
> disabled for the task. This will affect all confined processes that try
> to audit a capability message (so an process that has capability violation
> or is in learning mode and would have a capability violation).
> 
> The auditing code of capabilities, has a simple cache to reduce capability
> messages flooding the audit logs.  Checking and updating the cache
> disables kernel preemption.  One potential exit path does not properly
> put the per cpu var, thus not reenabling preemption.
> 
> Signed-off-by: John Johansen <john.johansen@canonical.com>

Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
>  ubuntu/apparmor/capability.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/ubuntu/apparmor/capability.c b/ubuntu/apparmor/capability.c
> index 65b91cf..5bb2eca 100644
> --- a/ubuntu/apparmor/capability.c
> +++ b/ubuntu/apparmor/capability.c
> @@ -72,6 +72,7 @@ static int aa_audit_caps(struct aa_profile *profile, struct aa_audit_caps *sa)
>  	/* Do simple duplicate message elimination */
>  	ent = &get_cpu_var(audit_cache);
>  	if (sa->base.task == ent->task && cap_raised(ent->caps, sa->cap)) {
> +		put_cpu_var(audit_cache);
>  		if (PROFILE_COMPLAIN(profile))
>  			return 0;
>  		return sa->base.error;
Andy Whitcroft Nov. 12, 2009, 11:19 a.m. UTC | #2
On Tue, Nov 10, 2009 at 10:29:11AM -0800, John Johansen wrote:
> BugLink: http://bugs.launchpad.net/bugs/479102
> 
> SRU Justification: Failing to put_cpu_var means that kernel preemption is
> disabled for the task. This will affect all confined processes that try
> to audit a capability message (so an process that has capability violation
> or is in learning mode and would have a capability violation).
> 
> The auditing code of capabilities, has a simple cache to reduce capability
> messages flooding the audit logs.  Checking and updating the cache
> disables kernel preemption.  One potential exit path does not properly
> put the per cpu var, thus not reenabling preemption.
> 
> Signed-off-by: John Johansen <john.johansen@canonical.com>
> ---
>  ubuntu/apparmor/capability.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/ubuntu/apparmor/capability.c b/ubuntu/apparmor/capability.c
> index 65b91cf..5bb2eca 100644
> --- a/ubuntu/apparmor/capability.c
> +++ b/ubuntu/apparmor/capability.c
> @@ -72,6 +72,7 @@ static int aa_audit_caps(struct aa_profile *profile, struct aa_audit_caps *sa)
>  	/* Do simple duplicate message elimination */
>  	ent = &get_cpu_var(audit_cache);
>  	if (sa->base.task == ent->task && cap_raised(ent->caps, sa->cap)) {
> +		put_cpu_var(audit_cache);
>  		if (PROFILE_COMPLAIN(profile))
>  			return 0;
>  		return sa->base.error;

This is a nice short section, looks right to me.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Stefan Bader Nov. 12, 2009, 1:38 p.m. UTC | #3
Applied
diff mbox

Patch

diff --git a/ubuntu/apparmor/capability.c b/ubuntu/apparmor/capability.c
index 65b91cf..5bb2eca 100644
--- a/ubuntu/apparmor/capability.c
+++ b/ubuntu/apparmor/capability.c
@@ -72,6 +72,7 @@  static int aa_audit_caps(struct aa_profile *profile, struct aa_audit_caps *sa)
 	/* Do simple duplicate message elimination */
 	ent = &get_cpu_var(audit_cache);
 	if (sa->base.task == ent->task && cap_raised(ent->caps, sa->cap)) {
+		put_cpu_var(audit_cache);
 		if (PROFILE_COMPLAIN(profile))
 			return 0;
 		return sa->base.error;