aa_replace_profiles() creates the raw_data/<rev> directory for the
loaddata and the profile directories for new profiles before the last
checks that can fail. When one of those checks fails, the fail path
releases the ns lock without removing what this load created.
The raw_data directory is named after the current ns revision, and the
revision is only bumped by a successful load. The stale directory is
only removed once the loaddata's last profile reference is dropped, from
a workqueue. Until then, every load fails to create raw_data/<rev> with
-EEXIST. If the new profiles are never freed, which is the case when
their dents are left behind pinning the proxy, that never happens and no
policy can be loaded until reboot.
While still holding the ns lock in the fail path, remove the profile
directories of the new profiles and, if this load created it, the
raw_data directory.
Fixes: 5d5182cae401 ("apparmor: move to per loaddata files, instead of
replicating in profiles")
Signed-off-by: Georgia Garcia <[email protected]>
---
security/apparmor/policy.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 397f9ff78aeb..f6f7cdcb6212 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -1191,6 +1191,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns,
struct aa_label *label,
struct aa_loaddata *rawdata_ent;
const char *op;
ssize_t count, error;
+ bool rawdata_created = false;
LIST_HEAD(lh);
op = mask & AA_MAY_REPLACE_POLICY ? OP_PROF_REPL : OP_PROF_LOAD;
@@ -1330,6 +1331,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns,
struct aa_label *label,
ent = NULL;
goto fail_lock;
}
+ rawdata_created = true;
}
list_for_each_entry(ent, &lh, list) {
if (!ent->old) {
@@ -1425,6 +1427,18 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns,
struct aa_label *label,
return udata_sz;
fail_lock:
+ /*
+ * Remove fs entries created by this load. Nothing else references
+ * them yet, and leaving them behind would leak the new profiles
+ * (their dents pin the proxy) and make the next load at the same,
+ * unbumped ns revision fail with -EEXIST on its raw_data dir.
+ */
+ list_for_each_entry(tmp, &lh, list) {
+ if (!tmp->old)
+ __aafs_profile_rmdir(tmp->new);
+ }
+ if (rawdata_created)
+ __aa_fs_remove_rawdata(udata);
mutex_unlock(&ns->lock);
/* audit cause of failure */
--
2.43.0
