mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2026-09-22 09:34:56 +02:00
apparmor: grab ns lock and refresh when looking up changehat child profiles
[ Upstream commit32e92764d6] There was a race condition involving change_hat and profile replacement in which replacement of the parent profile during a changehat operation could result in the list of children becoming empty and the changehat operation failing. To prevent this: - grab the namespace lock until we've built the hat transition, and - use aa_get_newest_profile to avoid using stale profile objects. Link: https://bugs.launchpad.net/bugs/2139664 Fixes:89dbf1962a("apparmor: move change_hat mediation to using labels") Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: Ryan Lee <ryan.lee@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
6a26de6c05
commit
5acb878db4
@@ -13,6 +13,7 @@
|
||||
#include <linux/fs.h>
|
||||
#include <linux/file.h>
|
||||
#include <linux/mount.h>
|
||||
#include <linux/mutex.h>
|
||||
#include <linux/syscalls.h>
|
||||
#include <linux/personality.h>
|
||||
#include <linux/xattr.h>
|
||||
@@ -1061,6 +1062,7 @@ static struct aa_label *change_hat(const struct cred *subj_cred,
|
||||
int count, int flags)
|
||||
{
|
||||
struct aa_profile *profile, *root, *hat = NULL;
|
||||
struct aa_ns *ns, *new_ns;
|
||||
struct aa_label *new;
|
||||
struct label_it it;
|
||||
bool sibling = false;
|
||||
@@ -1071,6 +1073,32 @@ static struct aa_label *change_hat(const struct cred *subj_cred,
|
||||
AA_BUG(!hats);
|
||||
AA_BUG(count < 1);
|
||||
|
||||
/*
|
||||
* Acquire the newest label and then hold the lock until we choose a
|
||||
* hat, so that profile replacement doesn't atomically truncate the
|
||||
* list of potential hats. Because we are getting the namespaces from
|
||||
* the profiles and label, we can rely on the namespaces being live
|
||||
* and avoid incrementing their refcounts while grabbing the lock.
|
||||
*/
|
||||
label = aa_get_label(label);
|
||||
ns = labels_ns(label);
|
||||
|
||||
retry:
|
||||
mutex_lock_nested(&ns->lock, ns->level);
|
||||
if (label_is_stale(label)) {
|
||||
new = aa_get_newest_label(label);
|
||||
new_ns = labels_ns(new);
|
||||
if (new_ns != ns) {
|
||||
aa_put_label(new);
|
||||
mutex_unlock(&ns->lock);
|
||||
ns = new_ns;
|
||||
label = new;
|
||||
goto retry;
|
||||
}
|
||||
aa_put_label(label);
|
||||
label = new;
|
||||
}
|
||||
|
||||
if (PROFILE_IS_HAT(labels_profile(label)))
|
||||
sibling = true;
|
||||
|
||||
@@ -1079,7 +1107,7 @@ static struct aa_label *change_hat(const struct cred *subj_cred,
|
||||
name = hats[i];
|
||||
label_for_each_in_ns(it, labels_ns(label), label, profile) {
|
||||
if (sibling && PROFILE_IS_HAT(profile)) {
|
||||
root = aa_get_profile_rcu(&profile->parent);
|
||||
root = aa_get_profile(profile->parent);
|
||||
} else if (!sibling && !PROFILE_IS_HAT(profile)) {
|
||||
root = aa_get_profile(profile);
|
||||
} else { /* conflicting change type */
|
||||
@@ -1139,6 +1167,7 @@ fail:
|
||||
GLOBAL_ROOT_UID, info, error);
|
||||
}
|
||||
}
|
||||
mutex_unlock(&ns->lock);
|
||||
return ERR_PTR(error);
|
||||
|
||||
build:
|
||||
@@ -1151,7 +1180,7 @@ build:
|
||||
error = -ENOMEM;
|
||||
goto fail;
|
||||
} /* else if (IS_ERR) build_change_hat has logged error so return new */
|
||||
|
||||
mutex_unlock(&ns->lock);
|
||||
return new;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user