Commit a125bcda authored by Linus Torvalds's avatar Linus Torvalds
Browse files

Merge tag 'apparmor-pr-2020-01-04' of...

Merge tag 'apparmor-pr-2020-01-04' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor

Pull apparmor fixes from John Johansen:

 - performance regression: only get a label reference if the fast path
   check fails

 - fix aa_xattrs_match() may sleep while holding a RCU lock

 - fix bind mounts aborting with -ENOMEM

* tag 'apparmor-pr-2020-01-04' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
  apparmor: fix aa_xattrs_match() may sleep while holding a RCU lock
  apparmor: only get a label reference if the fast path check fails
  apparmor: fix bind mounts aborting with -ENOMEM
parents c420ddda 8c62ed27
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -623,7 +623,7 @@ static __poll_t ns_revision_poll(struct file *file, poll_table *pt)

void __aa_bump_ns_revision(struct aa_ns *ns)
{
	ns->revision++;
	WRITE_ONCE(ns->revision, ns->revision + 1);
	wake_up_interruptible(&ns->wait);
}

+43 −39
Original line number Diff line number Diff line
@@ -317,6 +317,7 @@ static int aa_xattrs_match(const struct linux_binprm *bprm,

	if (!bprm || !profile->xattr_count)
		return 0;
	might_sleep();

	/* transition from exec match to xattr set */
	state = aa_dfa_null_transition(profile->xmatch, state);
@@ -361,10 +362,11 @@ out:
}

/**
 * __attach_match_ - find an attachment match
 * find_attach - do attachment search for unconfined processes
 * @bprm - binprm structure of transitioning task
 * @name - to match against  (NOT NULL)
 * @ns: the current namespace  (NOT NULL)
 * @head - profile list to walk  (NOT NULL)
 * @name - to match against  (NOT NULL)
 * @info - info message if there was an error (NOT NULL)
 *
 * Do a linear search on the profiles in the list.  There is a matching
@@ -374,12 +376,11 @@ out:
 *
 * Requires: @head not be shared or have appropriate locks held
 *
 * Returns: profile or NULL if no match found
 * Returns: label or NULL if no match found
 */
static struct aa_profile *__attach_match(const struct linux_binprm *bprm,
					 const char *name,
					 struct list_head *head,
					 const char **info)
static struct aa_label *find_attach(const struct linux_binprm *bprm,
				    struct aa_ns *ns, struct list_head *head,
				    const char *name, const char **info)
{
	int candidate_len = 0, candidate_xattrs = 0;
	bool conflict = false;
@@ -388,6 +389,8 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm,
	AA_BUG(!name);
	AA_BUG(!head);

	rcu_read_lock();
restart:
	list_for_each_entry_rcu(profile, head, base.list) {
		if (profile->label.flags & FLAG_NULL &&
		    &profile->label == ns_unconfined(profile->ns))
@@ -413,16 +416,32 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm,
			perm = dfa_user_allow(profile->xmatch, state);
			/* any accepting state means a valid match. */
			if (perm & MAY_EXEC) {
				int ret;
				int ret = 0;

				if (count < candidate_len)
					continue;

				ret = aa_xattrs_match(bprm, profile, state);
				/* Fail matching if the xattrs don't match */
				if (bprm && profile->xattr_count) {
					long rev = READ_ONCE(ns->revision);

					if (!aa_get_profile_not0(profile))
						goto restart;
					rcu_read_unlock();
					ret = aa_xattrs_match(bprm, profile,
							      state);
					rcu_read_lock();
					aa_put_profile(profile);
					if (rev !=
					    READ_ONCE(ns->revision))
						/* policy changed */
						goto restart;
					/*
					 * Fail matching if the xattrs don't
					 * match
					 */
					if (ret < 0)
						continue;

				}
				/*
				 * TODO: allow for more flexible best match
				 *
@@ -445,43 +464,28 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm,
				candidate_xattrs = ret;
				conflict = false;
			}
		} else if (!strcmp(profile->base.name, name))
		} else if (!strcmp(profile->base.name, name)) {
			/*
			 * old exact non-re match, without conditionals such
			 * as xattrs. no more searching required
			 */
			return profile;
			candidate = profile;
			goto out;
		}
	}

	if (conflict) {
	if (!candidate || conflict) {
		if (conflict)
			*info = "conflicting profile attachments";
		rcu_read_unlock();
		return NULL;
	}

	return candidate;
}

/**
 * find_attach - do attachment search for unconfined processes
 * @bprm - binprm structure of transitioning task
 * @ns: the current namespace  (NOT NULL)
 * @list: list to search  (NOT NULL)
 * @name: the executable name to match against  (NOT NULL)
 * @info: info message if there was an error
 *
 * Returns: label or NULL if no match found
 */
static struct aa_label *find_attach(const struct linux_binprm *bprm,
				    struct aa_ns *ns, struct list_head *list,
				    const char *name, const char **info)
{
	struct aa_profile *profile;

	rcu_read_lock();
	profile = aa_get_profile(__attach_match(bprm, name, list, info));
out:
	candidate = aa_get_newest_profile(candidate);
	rcu_read_unlock();

	return profile ? &profile->label : NULL;
	return &candidate->label;
}

static const char *next_name(int xtype, const char *name)
+8 −4
Original line number Diff line number Diff line
@@ -618,8 +618,7 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
	fctx = file_ctx(file);

	rcu_read_lock();
	flabel  = aa_get_newest_label(rcu_dereference(fctx->label));
	rcu_read_unlock();
	flabel  = rcu_dereference(fctx->label);
	AA_BUG(!flabel);

	/* revalidate access, if task is unconfined, or the cached cred
@@ -631,9 +630,13 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
	 */
	denied = request & ~fctx->allow;
	if (unconfined(label) || unconfined(flabel) ||
	    (!denied && aa_label_is_subset(flabel, label)))
	    (!denied && aa_label_is_subset(flabel, label))) {
		rcu_read_unlock();
		goto done;
	}

	flabel  = aa_get_newest_label(flabel);
	rcu_read_unlock();
	/* TODO: label cross check */

	if (file->f_path.mnt && path_mediated_fs(file->f_path.dentry))
@@ -643,8 +646,9 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
	else if (S_ISSOCK(file_inode(file)->i_mode))
		error = __file_sock_perm(op, label, flabel, file, request,
					 denied);
done:
	aa_put_label(flabel);

done:
	return error;
}

+1 −1
Original line number Diff line number Diff line
@@ -442,7 +442,7 @@ int aa_bind_mount(struct aa_label *label, const struct path *path,
	buffer = aa_get_buffer(false);
	old_buffer = aa_get_buffer(false);
	error = -ENOMEM;
	if (!buffer || old_buffer)
	if (!buffer || !old_buffer)
		goto out;

	error = fn_for_each_confined(label, profile,
+2 −2
Original line number Diff line number Diff line
@@ -1125,8 +1125,8 @@ ssize_t aa_remove_profiles(struct aa_ns *policy_ns, struct aa_label *subj,
	if (!name) {
		/* remove namespace - can only happen if fqname[0] == ':' */
		mutex_lock_nested(&ns->parent->lock, ns->level);
		__aa_remove_ns(ns);
		__aa_bump_ns_revision(ns);
		__aa_remove_ns(ns);
		mutex_unlock(&ns->parent->lock);
	} else {
		/* remove profile */
@@ -1138,9 +1138,9 @@ ssize_t aa_remove_profiles(struct aa_ns *policy_ns, struct aa_label *subj,
			goto fail_ns_lock;
		}
		name = profile->base.hname;
		__aa_bump_ns_revision(ns);
		__remove_profile(profile);
		__aa_labelset_update_subtree(ns);
		__aa_bump_ns_revision(ns);
		mutex_unlock(&ns->lock);
	}