[PATCH 09/11] AppArmor: address performance regression of replaced profile

Andy Whitcroft apw at canonical.com
Tue Apr 13 09:19:00 UTC 2010


On Tue, Apr 13, 2010 at 12:09:38AM -0700, john.johansen at canonical.com wrote:
> From: John Johansen <john.johansen at canonical.com>
> 
> OriginalAuthor: John Johansen <john.johansen at canonical.com>
> OriginalLocation: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparm$
> commit: 5b2ed5984571ca59570240f505dc90810bb56842
> BugLink: http://bugs.launchpad.net/bugs/549428
> 
> If a file has been opened under an old version of a profile (one that
> has been replaced) it is labeled with the original profile and the
> labeling is used to avoid performing revalidation (name lookup + permission
> check) on every file access.
> 
> Replacement changes the profile pointer so that the labeling check fails
> and revalidation must be performed.  This can cause a performance regression
> that is noticable on files that are accessed frequently.
> 
> Make sure to get the newest version of the cached file profile before
> comparing to current confinement profile.
> 
> Also, the permissions that were granted on open were not being stored
> in the file->cxt forcing a revalidation because the check to avoid
> revalidation also checks that the requested permissions are a subset of
> cached granted permissions.
> 
> So Ensure that the granted permissions are stored.
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> ---
>  security/apparmor/lsm.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index d1c1be0..56509ce 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -419,7 +419,7 @@ static int apparmor_dentry_open(struct file *file, const struct cred *cred)
>  		/* released by aa_free_file_context */
>  		fcxt->profile = aa_get_profile(profile);
>  		/* todo cache actual allowed permissions */
> -		fcxt->allowed = 0;
> +		fcxt->allowed = aa_map_file_to_perms(file);
>  	}
>  
>  	return error;
> @@ -448,7 +448,7 @@ static int apparmor_file_permission(struct file *file, int mask)
>  	 * TODO: cache profiles that have revalidated?
>  	 */
>  	struct aa_file_cxt *fcxt = file->f_security;
> -	struct aa_profile *profile, *fprofile = fcxt->profile;
> +	struct aa_profile *profile, *fprofile = aa_newest_version(fcxt->profile);
>  	int error = 0;
>  
>  	if (!fprofile || !file->f_path.mnt ||

Ok.  Seems to do what is claimed:

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

-apw




More information about the kernel-team mailing list