Commit 5593a733 authored by Linus Torvalds's avatar Linus Torvalds

Merge tag 'apparmor-pr-2021-11-10' of...

Merge tag 'apparmor-pr-2021-11-10' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor

Pull apparmor updates from John Johansen:
 "Features
   - use per file locks for transactional queries
   - update policy management capability checks to work with LSM stacking

  Bug Fixes:
   - check/put label on apparmor_sk_clone_security()
   - fix error check on update of label hname
   - fix introspection of of task mode for unconfined tasks

  Cleanups:
   - avoid -Wempty-body warning
   - remove duplicated 'Returns:' comments
   - fix doc warning
   - remove unneeded one-line hook wrappers
   - use struct_size() helper in kzalloc()
   - fix zero-length compiler warning in AA_BUG()
   - file.h: delete duplicated word
   - delete repeated words in comments
   - remove repeated declaration"

* tag 'apparmor-pr-2021-11-10' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
  apparmor: remove duplicated 'Returns:' comments
  apparmor: remove unneeded one-line hook wrappers
  apparmor: Use struct_size() helper in kzalloc()
  apparmor: fix zero-length compiler warning in AA_BUG()
  apparmor: use per file locks for transactional queries
  apparmor: fix doc warning
  apparmor: Remove the repeated declaration
  apparmor: avoid -Wempty-body warning
  apparmor: Fix internal policy capable check for policy management
  apparmor: fix error check
  security: apparmor: delete repeated words in comments
  security: apparmor: file.h: delete duplicated word
  apparmor: switch to apparmor to internal capable check for policy management
  apparmor: update policy capable checks to use a label
  apparmor: fix introspection of of task mode for unconfined tasks
  apparmor: check/put label on apparmor_sk_clone_security()
parents dbf49896 582122f1
......@@ -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);
......
......@@ -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.
......
......@@ -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])
......
......@@ -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...) \
......
......@@ -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 */
......@@ -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
......
......@@ -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,
......
......@@ -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)
......
......@@ -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);
......
......@@ -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
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment