diff options
| author | John Johansen <john.johansen@canonical.com> | 2026-02-24 10:20:02 -0800 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2026-03-13 17:26:04 +0100 |
| commit | 763e838adc3c7ec5a7df2990ce84cad951e42721 (patch) | |
| tree | 9c113860f639b5ce382713c17e271d47b381d861 /security/apparmor/policy.c | |
| parent | 1ff4857fac56ac5a90ee63b24db05fa5e91a45aa (diff) | |
apparmor: fix race on rawdata dereference
commit a0b7091c4de45a7325c8780e6934a894f92ac86b upstream.
There is a race condition that leads to a use-after-free situation:
because the rawdata inodes are not refcounted, an attacker can start
open()ing one of the rawdata files, and at the same time remove the
last reference to this rawdata (by removing the corresponding profile,
for example), which frees its struct aa_loaddata; as a result, when
seq_rawdata_open() is reached, i_private is a dangling pointer and
freed memory is accessed.
The rawdata inodes weren't refcounted to avoid a circular refcount and
were supposed to be held by the profile rawdata reference. However
during profile removal there is a window where the vfs and profile
destruction race, resulting in the use after free.
Fix this by moving to a double refcount scheme. Where the profile
refcount on rawdata is used to break the circular dependency. Allowing
for freeing of the rawdata once all inode references to the rawdata
are put.
Fixes: 5d5182cae401 ("apparmor: move to per loaddata files, instead of replicating in profiles")
Reported-by: Qualys Security Advisory <qsa@qualys.com>
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Reviewed-by: Maxime Bélair <maxime.belair@canonical.com>
Reviewed-by: Cengiz Can <cengiz.can@canonical.com>
Tested-by: Salvatore Bonaccorso <carnil@debian.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'security/apparmor/policy.c')
| -rw-r--r-- | security/apparmor/policy.c | 12 |
1 files changed, 6 insertions, 6 deletions
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 3623ec620cba..b92db1b2f26e 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -336,7 +336,7 @@ void aa_free_profile(struct aa_profile *profile) } kfree_sensitive(profile->hash); - aa_put_loaddata(profile->rawdata); + aa_put_profile_loaddata(profile->rawdata); aa_label_destroy(&profile->label); kfree_sensitive(profile); @@ -1154,7 +1154,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, LIST_HEAD(lh); op = mask & AA_MAY_REPLACE_POLICY ? OP_PROF_REPL : OP_PROF_LOAD; - aa_get_loaddata(udata); + aa_get_profile_loaddata(udata); /* released below */ error = aa_unpack(udata, &lh, &ns_name); if (error) @@ -1206,10 +1206,10 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, if (aa_rawdata_eq(rawdata_ent, udata)) { struct aa_loaddata *tmp; - tmp = __aa_get_loaddata(rawdata_ent); + tmp = aa_get_profile_loaddata(rawdata_ent); /* check we didn't fail the race */ if (tmp) { - aa_put_loaddata(udata); + aa_put_profile_loaddata(udata); udata = tmp; break; } @@ -1222,7 +1222,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, struct aa_profile *p; if (aa_g_export_binary) - ent->new->rawdata = aa_get_loaddata(udata); + ent->new->rawdata = aa_get_profile_loaddata(udata); error = __lookup_replace(ns, ent->new->base.hname, !(mask & AA_MAY_REPLACE_POLICY), &ent->old, &info); @@ -1355,7 +1355,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, out: aa_put_ns(ns); - aa_put_loaddata(udata); + aa_put_profile_loaddata(udata); kfree(ns_name); if (error) |