| Message ID | b8b6a59e-4483-addf-85e6-5d45ea2d6364@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Xenial,Zesty] UBUNTU: SAUCE: fix oops when disabled and module parameters, are accessed | expand |
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@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@canonical.com>
On 01.09.2017 09: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@canonical.com> Acked-by: Stefan Bader <stefan.bader@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) { >
On 01.09.2017 09: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@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) { > This patch is already applied in Zesty (and was even before release) as UBUNTU: SAUCE: apparmor: fix parameters so that the permission test is bypas sed at boot BugLink: http://bugs.launchpad.net/bugs/1678048
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) {
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@canonical.com> --- security/apparmor/lsm.c | 52 +++++++++++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 21 deletions(-)