[apparmor] [PATCH 5/6] libapparmor: Allow creating a policy_cache with a NULL kernel_features

[Steve Beattie]

On Wed, Mar 25, 2015 at 05:37:20PM -0500, Tyler Hicks wrote:
> The most common case when creating an aa_policy_cache object will be to
> do so while using the current kernel's feature set for the
> kernel_features parameter. Rather than have callers instantiate their
> own aa_features object in this situation, aa_policy_cache_new() should
> do it for them if they specify NULL for the kernel_features parameter.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>

I like this change a lot. Acked-by: Steve Beattie <steve at nxnw.org>.
Thanks.

> ---
>  libraries/libapparmor/src/policy_cache.c    | 17 +++++++++++---
>  tests/regression/apparmor/aa_policy_cache.c | 36 ++++-------------------------
>  2 files changed, 18 insertions(+), 35 deletions(-)
> 
> diff --git a/libraries/libapparmor/src/policy_cache.c b/libraries/libapparmor/src/policy_cache.c
> index 44eb34d..779c91d 100644
> --- a/libraries/libapparmor/src/policy_cache.c
> +++ b/libraries/libapparmor/src/policy_cache.c
> @@ -129,7 +129,8 @@ static int replace_all_cb(DIR *dir unused, const char *name, struct stat *st,
>   * aa_policy_cache_new - create a new policy_cache from a path
>   * @policy_cache: will point to the address of an allocated and initialized
>   *                aa_policy_cache_new object upon success
> - * @kernel_features: features representing a kernel
> + * @kernel_features: features representing a kernel (may be NULL if you want to
> + *                   use the features of the currently running kernel)
>   * @path: path to the policy cache
>   * @create: true if the cache should be created if it doesn't already exist
>   *
> @@ -170,6 +171,17 @@ int aa_policy_cache_new(aa_policy_cache **policy_cache,
>  		return -1;
>  	}
>  
> +	if (kernel_features) {
> +		aa_features_ref(kernel_features);
> +	} else if (aa_features_new_from_kernel(&kernel_features) == -1) {
> +		int save = errno;
> +
> +		aa_policy_cache_unref(pc);
> +		errno = save;
> +		return -1;
> +	}
> +	pc->kernel_features = kernel_features;
> +
>  	if (init_cache_features(pc, kernel_features, create)) {
>  		int save = errno;
>  
> @@ -178,7 +190,6 @@ int aa_policy_cache_new(aa_policy_cache **policy_cache,
>  		return -1;
>  	}
>  
> -	pc->kernel_features = aa_features_ref(kernel_features);
>  	*policy_cache = pc;
>  
>  	return 0;
> @@ -203,8 +214,8 @@ aa_policy_cache *aa_policy_cache_ref(aa_policy_cache *policy_cache)
>  void aa_policy_cache_unref(aa_policy_cache *policy_cache)
>  {
>  	if (policy_cache && atomic_dec_and_test(&policy_cache->ref_count)) {
> -		aa_features_unref(policy_cache->kernel_features);
>  		aa_features_unref(policy_cache->features);
> +		aa_features_unref(policy_cache->kernel_features);
>  		free(policy_cache->features_path);
>  		free(policy_cache->path);
>  		free(policy_cache);
> diff --git a/tests/regression/apparmor/aa_policy_cache.c b/tests/regression/apparmor/aa_policy_cache.c
> index cb4bc71..03f7b5b 100644
> --- a/tests/regression/apparmor/aa_policy_cache.c
> +++ b/tests/regression/apparmor/aa_policy_cache.c
> @@ -47,16 +47,10 @@ static void usage(const char *prog)
>  
>  static int test_make_valid(const char *path)
>  {
> -	aa_features *features = NULL;
>  	aa_policy_cache *policy_cache = NULL;
>  	int rc = 1;
>  
> -	if (aa_features_new_from_kernel(&features)) {
> -		perror("FAIL - aa_features_new_from_kernel");
> -		goto out;
> -	}
> -
> -	if (aa_policy_cache_new(&policy_cache, features, path, false)) {
> +	if (aa_policy_cache_new(&policy_cache, NULL, path, false)) {
>  		perror("FAIL - aa_policy_cache_new");
>  		goto out;
>  	}
> @@ -68,23 +62,16 @@ static int test_make_valid(const char *path)
>  
>  	rc = 0;
>  out:
> -	aa_features_unref(features);
>  	aa_policy_cache_unref(policy_cache);
>  	return rc;
>  }
>  
>  static int test_is_valid(const char *path)
>  {
> -	aa_features *features = NULL;
>  	aa_policy_cache *policy_cache = NULL;
>  	int rc = 1;
>  
> -	if (aa_features_new_from_kernel(&features)) {
> -		perror("FAIL - aa_features_new_from_kernel");
> -		goto out;
> -	}
> -
> -	if (aa_policy_cache_new(&policy_cache, features, path, false)) {
> +	if (aa_policy_cache_new(&policy_cache, NULL, path, false)) {
>  		perror("FAIL - aa_policy_cache_new");
>  		goto out;
>  	}
> @@ -97,30 +84,22 @@ static int test_is_valid(const char *path)
>  
>  	rc = 0;
>  out:
> -	aa_features_unref(features);
>  	aa_policy_cache_unref(policy_cache);
>  	return rc;
>  }
>  
>  static int test_new(const char *path, bool create)
>  {
> -	aa_features *features = NULL;
>  	aa_policy_cache *policy_cache = NULL;
>  	int rc = 1;
>  
> -	if (aa_features_new_from_kernel(&features)) {
> -		perror("FAIL - aa_features_new_from_kernel");
> -		goto out;
> -	}
> -
> -	if (aa_policy_cache_new(&policy_cache, features, path, create)) {
> +	if (aa_policy_cache_new(&policy_cache, NULL, path, create)) {
>  		perror("FAIL - aa_policy_cache_new");
>  		goto out;
>  	}
>  
>  	rc = 0;
>  out:
> -	aa_features_unref(features);
>  	aa_policy_cache_unref(policy_cache);
>  	return rc;
>  }
> @@ -169,16 +148,10 @@ out:
>  
>  static int test_replace_all(const char *path)
>  {
> -	aa_features *features = NULL;
>  	aa_policy_cache *policy_cache = NULL;
>  	int rc = 1;
>  
> -	if (aa_features_new_from_kernel(&features)) {
> -		perror("FAIL - aa_features_new_from_kernel");
> -		goto out;
> -	}
> -
> -	if (aa_policy_cache_new(&policy_cache, features, path, false)) {
> +	if (aa_policy_cache_new(&policy_cache, NULL, path, false)) {
>  		perror("FAIL - aa_policy_cache_new");
>  		goto out;
>  	}
> @@ -190,7 +163,6 @@ static int test_replace_all(const char *path)
>  
>  	rc = 0;
>  out:
> -	aa_features_unref(features);
>  	aa_policy_cache_unref(policy_cache);
>  	return rc;
>  }

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150326/a5f8f802/attachment.pgp>