ACK: [PATCH][Xenial][Zesty] UBUNTU: SAUCE: fix oops when disabled and module parameters, are accessed

Colin Ian King colin.king at canonical.com
Tue Sep 5 12:13:34 UTC 2017


On 01/09/17 08:05, John Johansen wrote:
> The virtualization of apparmor module parameters failed to take into
> account the parameters being accessed when apparmor is not enabled
> in some cases.
> 
> It also failed to take into account that policy_admin_capable checks
> should not be applied to parameters specified at kernel boot as this
> is the callback is used before apparmor is initialized.
> 
> BugLink: http://bugs.launchpad.net/bugs/1626984
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> ---
>  security/apparmor/lsm.c | 52 +++++++++++++++++++++++++++++--------------------
>  1 file changed, 31 insertions(+), 21 deletions(-)
> 
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 70617e50a0d4..7951c3dc9393 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -41,7 +41,7 @@
>  #include "include/mount.h"
>  
>  /* Flag indicating whether initialization completed */
> -int apparmor_initialized __initdata;
> +int apparmor_initialized;
>  
>  DEFINE_PER_CPU(struct aa_buffers, aa_buffers);
>  
> @@ -1409,74 +1409,83 @@ __setup("apparmor=", apparmor_enabled_setup);
>  /* set global flag turning off the ability to load policy */
>  static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp)
>  {
> -	if (!policy_admin_capable(NULL))
> +	if (!apparmor_enabled)
> +		return -EINVAL;
> +	if (apparmor_initialized && !policy_admin_capable(NULL))
>  		return -EPERM;
>  	return param_set_bool(val, kp);
>  }
>  
>  static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp)
>  {
> -	if (!policy_view_capable(NULL))
> -		return -EPERM;
>  	if (!apparmor_enabled)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_view_capable(NULL))
> +		return -EPERM;
>  	return param_get_bool(buffer, kp);
>  }
>  
>  static int param_set_aabool(const char *val, const struct kernel_param *kp)
>  {
> -	if (!policy_admin_capable(NULL))
> -		return -EPERM;
>  	if (!apparmor_enabled)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_admin_capable(NULL))
> +		return -EPERM;
>  	return param_set_bool(val, kp);
>  }
>  
>  static int param_get_aabool(char *buffer, const struct kernel_param *kp)
>  {
> -	if (!policy_view_capable(NULL))
> -		return -EPERM;
>  	if (!apparmor_enabled)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_view_capable(NULL))
> +		return -EPERM;
>  	return param_get_bool(buffer, kp);
>  }
>  
>  static int param_set_aauint(const char *val, const struct kernel_param *kp)
>  {
> -	if (!policy_admin_capable(NULL))
> -		return -EPERM;
> +	int error;
> +
>  	if (!apparmor_enabled)
>  		return -EINVAL;
> -	return param_set_uint(val, kp);
> +	if (apparmor_initialized && !policy_admin_capable(NULL))
> +		return -EPERM;
> +
> +	error = param_set_uint(val, kp);
> +	pr_info("AppArmor: buffer size set to %d bytes\n", aa_g_path_max);
> +
> +	return error;
>  }
>  
>  static int param_get_aauint(char *buffer, const struct kernel_param *kp)
>  {
> -	if (!policy_view_capable(NULL))
> -		return -EPERM;
>  	if (!apparmor_enabled)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_view_capable(NULL))
> +		return -EPERM;
>  	return param_get_uint(buffer, kp);
>  }
>  
>  static int param_get_audit(char *buffer, struct kernel_param *kp)
>  {
> -	if (!policy_view_capable(NULL))
> -		return -EPERM;
>  	if (!apparmor_enabled)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_view_capable(NULL))
> +		return -EPERM;
>  	return sprintf(buffer, "%s", audit_mode_names[aa_g_audit]);
>  }
>  
>  static int param_set_audit(const char *val, struct kernel_param *kp)
>  {
>  	int i;
> -	if (!policy_admin_capable(NULL))
> -		return -EPERM;
> +
>  	if (!apparmor_enabled)
>  		return -EINVAL;
>  	if (!val)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_admin_capable(NULL))
> +		return -EPERM;
>  
>  	for (i = 0; i < AUDIT_MAX_INDEX; i++) {
>  		if (strcmp(val, audit_mode_names[i]) == 0) {
> @@ -1490,10 +1499,10 @@ static int param_set_audit(const char *val, struct kernel_param *kp)
>  
>  static int param_get_mode(char *buffer, struct kernel_param *kp)
>  {
> -	if (!policy_view_capable(NULL))
> -		return -EPERM;
>  	if (!apparmor_enabled)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_view_capable(NULL))
> +		return -EPERM;
>  
>  	return sprintf(buffer, "%s", aa_profile_mode_names[aa_g_profile_mode]);
>  }
> @@ -1501,12 +1510,13 @@ static int param_get_mode(char *buffer, struct kernel_param *kp)
>  static int param_set_mode(const char *val, struct kernel_param *kp)
>  {
>  	int i;
> -	if (!policy_admin_capable(NULL))
> -		return -EPERM;
> +
>  	if (!apparmor_enabled)
>  		return -EINVAL;
>  	if (!val)
>  		return -EINVAL;
> +	if (apparmor_initialized && !policy_admin_capable(NULL))
> +		return -EPERM;
>  
>  	for (i = 0; i < APPARMOR_MODE_NAMES_MAX_INDEX; i++) {
>  		if (strcmp(val, aa_profile_mode_names[i]) == 0) {
> 
Acked-by: Colin Ian King <colin.king at canonical.com>




More information about the kernel-team mailing list