diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 2ee3b3d29f10..0797edb2fb3d 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -812,8 +812,6 @@ struct multi_transaction { }; #define MULTI_TRANSACTION_LIMIT (PAGE_SIZE - sizeof(struct multi_transaction)) -/* TODO: replace with per file lock */ -static DEFINE_SPINLOCK(multi_transaction_lock); static void multi_transaction_kref(struct kref *kref) { @@ -847,10 +845,10 @@ static void multi_transaction_set(struct file *file, AA_BUG(n > MULTI_TRANSACTION_LIMIT); new->size = n; - spin_lock(&multi_transaction_lock); + spin_lock(&file->f_lock); old = (struct multi_transaction *) file->private_data; file->private_data = new; - spin_unlock(&multi_transaction_lock); + spin_unlock(&file->f_lock); put_multi_transaction(old); } @@ -879,9 +877,10 @@ static ssize_t multi_transaction_read(struct file *file, char __user *buf, struct multi_transaction *t; ssize_t ret; - spin_lock(&multi_transaction_lock); + spin_lock(&file->f_lock); t = get_multi_transaction(file->private_data); - spin_unlock(&multi_transaction_lock); + spin_unlock(&file->f_lock); + if (!t) return 0; @@ -1358,7 +1357,7 @@ static int rawdata_open(struct inode *inode, struct file *file) struct aa_loaddata *loaddata; struct rawdata_f_data *private; - if (!policy_view_capable(NULL)) + if (!aa_current_policy_view_capable(NULL)) return -EACCES; loaddata = __aa_get_loaddata(inode->i_private); @@ -2114,7 +2113,7 @@ static struct aa_profile *__first_profile(struct aa_ns *root, /** * __next_profile - step to the next profile in a profile tree - * @profile: current profile in tree (NOT NULL) + * @p: current profile in tree (NOT NULL) * * Perform a depth first traversal on the profile tree in a namespace * @@ -2265,7 +2264,7 @@ static const struct seq_operations aa_sfs_profiles_op = { static int profiles_open(struct inode *inode, struct file *file) { - if (!policy_view_capable(NULL)) + if (!aa_current_policy_view_capable(NULL)) return -EACCES; return seq_open(file, &aa_sfs_profiles_op); diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h index d4f8948517d9..7517605a183d 100644 --- a/security/apparmor/include/file.h +++ b/security/apparmor/include/file.h @@ -167,7 +167,7 @@ int aa_audit_file(struct aa_profile *profile, struct aa_perms *perms, * @perms: permission table indexed by the matched state accept entry of @dfa * @trans: transition table for indexed by named x transitions * - * File permission are determined by matching a path against @dfa and then + * File permission are determined by matching a path against @dfa and * then using the value of the accept entry for the matching state as * an index into @perms. If a named exec transition is required it is * looked up in the transition table. diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h index 1e90384b1523..9101c2c76d9e 100644 --- a/security/apparmor/include/label.h +++ b/security/apparmor/include/label.h @@ -77,10 +77,6 @@ struct aa_labelset { #define __labelset_for_each(LS, N) \ for ((N) = rb_first(&(LS)->root); (N); (N) = rb_next(N)) -void aa_labelset_destroy(struct aa_labelset *ls); -void aa_labelset_init(struct aa_labelset *ls); - - enum label_flags { FLAG_HAT = 1, /* profile is a hat */ FLAG_UNCONFINED = 2, /* label unconfined only if all */ @@ -148,6 +144,7 @@ do { \ #define __label_make_stale(X) ((X)->flags |= FLAG_STALE) #define labels_ns(X) (vec_ns(&((X)->vec[0]), (X)->size)) #define labels_set(X) (&labels_ns(X)->labels) +#define labels_view(X) labels_ns(X) #define labels_profile(X) ((X)->vec[(X)->size - 1]) diff --git a/security/apparmor/include/lib.h b/security/apparmor/include/lib.h index 7d27db740bc2..e2e8df0c6f1c 100644 --- a/security/apparmor/include/lib.h +++ b/security/apparmor/include/lib.h @@ -31,12 +31,17 @@ #define AA_WARN(X) WARN((X), "APPARMOR WARN %s: %s\n", __func__, #X) -#define AA_BUG(X, args...) AA_BUG_FMT((X), "" args) +#define AA_BUG(X, args...) \ + do { \ + _Pragma("GCC diagnostic ignored \"-Wformat-zero-length\""); \ + AA_BUG_FMT((X), "" args); \ + _Pragma("GCC diagnostic warning \"-Wformat-zero-length\""); \ + } while (0) #ifdef CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS #define AA_BUG_FMT(X, fmt, args...) \ WARN((X), "AppArmor WARN %s: (" #X "): " fmt, __func__, ##args) #else -#define AA_BUG_FMT(X, fmt, args...) +#define AA_BUG_FMT(X, fmt, args...) no_printk(fmt, ##args) #endif #define AA_ERROR(fmt, args...) \ diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index b5b4b8190e65..cb5ef21991b7 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -301,9 +301,11 @@ static inline int AUDIT_MODE(struct aa_profile *profile) return profile->audit; } -bool policy_view_capable(struct aa_ns *ns); -bool policy_admin_capable(struct aa_ns *ns); +bool aa_policy_view_capable(struct aa_label *label, struct aa_ns *ns); +bool aa_policy_admin_capable(struct aa_label *label, struct aa_ns *ns); int aa_may_manage_policy(struct aa_label *label, struct aa_ns *ns, u32 mask); +bool aa_current_policy_view_capable(struct aa_ns *ns); +bool aa_current_policy_admin_capable(struct aa_ns *ns); #endif /* __AA_POLICY_H */ diff --git a/security/apparmor/label.c b/security/apparmor/label.c index e68bcedca976..0b0265da1926 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -425,8 +425,7 @@ struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp) AA_BUG(size < 1); /* + 1 for null terminator entry on vec */ - new = kzalloc(sizeof(*new) + sizeof(struct aa_profile *) * (size + 1), - gfp); + new = kzalloc(struct_size(new, vec, size + 1), gfp); AA_DEBUG("%s (%p)\n", __func__, new); if (!new) goto fail; @@ -1454,7 +1453,7 @@ bool aa_update_label_name(struct aa_ns *ns, struct aa_label *label, gfp_t gfp) if (label->hname || labels_ns(label) != ns) return res; - if (aa_label_acntsxprint(&name, ns, label, FLAGS_NONE, gfp) == -1) + if (aa_label_acntsxprint(&name, ns, label, FLAGS_NONE, gfp) < 0) return res; ls = labels_set(label); @@ -1704,7 +1703,7 @@ int aa_label_asxprint(char **strp, struct aa_ns *ns, struct aa_label *label, /** * aa_label_acntsxprint - allocate a __counted string buffer and print label - * @strp: buffer to write to. (MAY BE NULL if @size == 0) + * @strp: buffer to write to. * @ns: namespace profile is being viewed from * @label: label to view (NOT NULL) * @flags: flags controlling what label info is printed diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index f72406fe1bf2..0d6585056f3d 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1402,7 +1402,7 @@ static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_admin_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_admin_capable(NULL)) return -EPERM; return param_set_bool(val, kp); } @@ -1411,7 +1411,7 @@ static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp) { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_view_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_view_capable(NULL)) return -EPERM; return param_get_bool(buffer, kp); } @@ -1420,7 +1420,7 @@ static int param_set_aabool(const char *val, const struct kernel_param *kp) { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_admin_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_admin_capable(NULL)) return -EPERM; return param_set_bool(val, kp); } @@ -1429,7 +1429,7 @@ static int param_get_aabool(char *buffer, const struct kernel_param *kp) { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_view_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_view_capable(NULL)) return -EPERM; return param_get_bool(buffer, kp); } @@ -1455,7 +1455,7 @@ static int param_get_aauint(char *buffer, const struct kernel_param *kp) { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_view_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_view_capable(NULL)) return -EPERM; return param_get_uint(buffer, kp); } @@ -1526,7 +1526,7 @@ static int param_get_aacompressionlevel(char *buffer, { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_view_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_view_capable(NULL)) return -EPERM; return param_get_int(buffer, kp); } @@ -1535,7 +1535,7 @@ static int param_get_audit(char *buffer, const struct kernel_param *kp) { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_view_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_view_capable(NULL)) return -EPERM; return sprintf(buffer, "%s", audit_mode_names[aa_g_audit]); } @@ -1548,7 +1548,7 @@ static int param_set_audit(const char *val, const struct kernel_param *kp) return -EINVAL; if (!val) return -EINVAL; - if (apparmor_initialized && !policy_admin_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_admin_capable(NULL)) return -EPERM; i = match_string(audit_mode_names, AUDIT_MAX_INDEX, val); @@ -1563,7 +1563,7 @@ static int param_get_mode(char *buffer, const struct kernel_param *kp) { if (!apparmor_enabled) return -EINVAL; - if (apparmor_initialized && !policy_view_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_view_capable(NULL)) return -EPERM; return sprintf(buffer, "%s", aa_profile_mode_names[aa_g_profile_mode]); @@ -1577,7 +1577,7 @@ static int param_set_mode(const char *val, const struct kernel_param *kp) return -EINVAL; if (!val) return -EINVAL; - if (apparmor_initialized && !policy_admin_capable(NULL)) + if (apparmor_initialized && !aa_current_policy_admin_capable(NULL)) return -EPERM; i = match_string(aa_profile_mode_names, APPARMOR_MODE_NAMES_MAX_INDEX, @@ -1713,7 +1713,7 @@ static int __init alloc_buffers(void) static int apparmor_dointvec(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { - if (!policy_admin_capable(NULL)) + if (!aa_current_policy_admin_capable(NULL)) return -EPERM; if (!apparmor_enabled) return -EINVAL; @@ -1773,32 +1773,16 @@ static unsigned int apparmor_ip_postroute(void *priv, } -static unsigned int apparmor_ipv4_postroute(void *priv, - struct sk_buff *skb, - const struct nf_hook_state *state) -{ - return apparmor_ip_postroute(priv, skb, state); -} - -#if IS_ENABLED(CONFIG_IPV6) -static unsigned int apparmor_ipv6_postroute(void *priv, - struct sk_buff *skb, - const struct nf_hook_state *state) -{ - return apparmor_ip_postroute(priv, skb, state); -} -#endif - static const struct nf_hook_ops apparmor_nf_ops[] = { { - .hook = apparmor_ipv4_postroute, + .hook = apparmor_ip_postroute, .pf = NFPROTO_IPV4, .hooknum = NF_INET_POST_ROUTING, .priority = NF_IP_PRI_SELINUX_FIRST, }, #if IS_ENABLED(CONFIG_IPV6) { - .hook = apparmor_ipv6_postroute, + .hook = apparmor_ip_postroute, .pf = NFPROTO_IPV6, .hooknum = NF_INET_POST_ROUTING, .priority = NF_IP6_PRI_SELINUX_FIRST, diff --git a/security/apparmor/path.c b/security/apparmor/path.c index b02dfdbff7cd..45ec994b558d 100644 --- a/security/apparmor/path.c +++ b/security/apparmor/path.c @@ -83,7 +83,7 @@ static int disconnect(const struct path *path, char *buf, char **name, * * Returns: %0 else error code if path lookup fails * When no error the path name is returned in @name which points to - * to a position in @buf + * a position in @buf */ static int d_namespace_path(const struct path *path, char *buf, char **name, int flags, const char *disconnected) diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 4c010c9a6af1..b0cbc4906cb3 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -260,8 +260,7 @@ struct aa_profile *aa_alloc_profile(const char *hname, struct aa_proxy *proxy, struct aa_profile *profile; /* freed by free_profile - usually through aa_put_profile */ - profile = kzalloc(sizeof(*profile) + sizeof(struct aa_profile *) * 2, - gfp); + profile = kzalloc(struct_size(profile, label.vec, 2), gfp); if (!profile) return NULL; @@ -632,18 +631,35 @@ static int audit_policy(struct aa_label *label, const char *op, return error; } +/* don't call out to other LSMs in the stack for apparmor policy admin + * permissions + */ +static int policy_ns_capable(struct aa_label *label, + struct user_namespace *userns, int cap) +{ + int err; + + /* check for MAC_ADMIN cap in cred */ + err = cap_capable(current_cred(), userns, cap, CAP_OPT_NONE); + if (!err) + err = aa_capable(label, cap, CAP_OPT_NONE); + + return err; +} + /** - * policy_view_capable - check if viewing policy in at @ns is allowed - * ns: namespace being viewed by current task (may be NULL) + * aa_policy_view_capable - check if viewing policy in at @ns is allowed + * label: label that is trying to view policy in ns + * ns: namespace being viewed by @label (may be NULL if @label's ns) * Returns: true if viewing policy is allowed * * If @ns is NULL then the namespace being viewed is assumed to be the * tasks current namespace. */ -bool policy_view_capable(struct aa_ns *ns) +bool aa_policy_view_capable(struct aa_label *label, struct aa_ns *ns) { struct user_namespace *user_ns = current_user_ns(); - struct aa_ns *view_ns = aa_get_current_ns(); + struct aa_ns *view_ns = labels_view(label); bool root_in_user_ns = uid_eq(current_euid(), make_kuid(user_ns, 0)) || in_egroup_p(make_kgid(user_ns, 0)); bool response = false; @@ -655,20 +671,44 @@ bool policy_view_capable(struct aa_ns *ns) (unprivileged_userns_apparmor_policy != 0 && user_ns->level == view_ns->level))) response = true; - aa_put_ns(view_ns); return response; } -bool policy_admin_capable(struct aa_ns *ns) +bool aa_policy_admin_capable(struct aa_label *label, struct aa_ns *ns) { struct user_namespace *user_ns = current_user_ns(); - bool capable = ns_capable(user_ns, CAP_MAC_ADMIN); + bool capable = policy_ns_capable(label, user_ns, CAP_MAC_ADMIN) == 0; AA_DEBUG("cap_mac_admin? %d\n", capable); AA_DEBUG("policy locked? %d\n", aa_g_lock_policy); - return policy_view_capable(ns) && capable && !aa_g_lock_policy; + return aa_policy_view_capable(label, ns) && capable && + !aa_g_lock_policy; +} + +bool aa_current_policy_view_capable(struct aa_ns *ns) +{ + struct aa_label *label; + bool res; + + label = __begin_current_label_crit_section(); + res = aa_policy_view_capable(label, ns); + __end_current_label_crit_section(label); + + return res; +} + +bool aa_current_policy_admin_capable(struct aa_ns *ns) +{ + struct aa_label *label; + bool res; + + label = __begin_current_label_crit_section(); + res = aa_policy_admin_capable(label, ns); + __end_current_label_crit_section(label); + + return res; } /** @@ -694,7 +734,7 @@ int aa_may_manage_policy(struct aa_label *label, struct aa_ns *ns, u32 mask) return audit_policy(label, op, NULL, NULL, "policy_locked", -EACCES); - if (!policy_admin_capable(ns)) + if (!aa_policy_admin_capable(label, ns)) return audit_policy(label, op, NULL, NULL, "not policy admin", -EACCES); diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 4e1f96b216a8..0acca6f2a93f 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -39,7 +39,7 @@ /* * The AppArmor interface treats data as a type byte followed by the - * actual data. The interface has the notion of a a named entry + * actual data. The interface has the notion of a named entry * which has a name (AA_NAME typecode followed by name string) followed by * the entries typecode and data. Named types allow for optional * elements and extensions to be added and tested for without breaking diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c index c929bf4a3df1..fde332e0ea7d 100644 --- a/security/apparmor/procattr.c +++ b/security/apparmor/procattr.c @@ -21,8 +21,6 @@ * @profile: the profile to print profile info about (NOT NULL) * @string: Returns - string containing the profile info (NOT NULL) * - * Returns: length of @string on success else error on failure - * * Requires: profile != NULL * * Creates a string containing the namespace_name://profile_name for