mirror of
https://gitlab.com/apparmor/apparmor.git
synced 2025-03-04 16:35:02 +01:00
275 lines
8.3 KiB
Diff
275 lines
8.3 KiB
Diff
Index: b/security/apparmor/inline.h
|
|
===================================================================
|
|
--- a/security/apparmor/inline.h
|
|
+++ b/security/apparmor/inline.h
|
|
@@ -75,9 +75,10 @@ aa_change_task_context(struct task_struc
|
|
{
|
|
struct aa_task_context *old_cxt = aa_task_context(task);
|
|
|
|
- if (old_cxt)
|
|
+ if (old_cxt) {
|
|
list_del_init(&old_cxt->list);
|
|
-
|
|
+ call_rcu(&old_cxt->rcu, free_aa_task_context_rcu_callback);
|
|
+ }
|
|
if (new_cxt) {
|
|
new_cxt->caps_logged = CAP_EMPTY_SET;
|
|
new_cxt->hat_magic = hat_magic;
|
|
@@ -85,12 +86,10 @@ aa_change_task_context(struct task_struc
|
|
new_cxt->profile = aa_dup_profile(profile);
|
|
list_move(&new_cxt->list, &profile->parent->task_contexts);
|
|
}
|
|
-
|
|
rcu_assign_pointer(task->security, new_cxt);
|
|
}
|
|
|
|
-static inline struct aa_task_context *
|
|
-aa_alloc_task_context(struct task_struct *task)
|
|
+static inline struct aa_task_context *aa_alloc_task_context(void)
|
|
{
|
|
struct aa_task_context *cxt;
|
|
|
|
@@ -98,7 +97,6 @@ aa_alloc_task_context(struct task_struct
|
|
if (cxt) {
|
|
INIT_LIST_HEAD(&cxt->list);
|
|
INIT_RCU_HEAD(&cxt->rcu);
|
|
- cxt->task = task;
|
|
}
|
|
|
|
return cxt;
|
|
@@ -112,12 +110,6 @@ static inline void aa_free_task_context(
|
|
}
|
|
}
|
|
|
|
-static inline void aa_free_task_context_rcu(struct aa_task_context *cxt)
|
|
-{
|
|
- if (cxt)
|
|
- call_rcu(&cxt->rcu, free_aa_task_context_rcu_callback);
|
|
-}
|
|
-
|
|
/**
|
|
* alloc_aa_profile - Allocate, initialize and return a new zeroed profile.
|
|
* Returns NULL on failure.
|
|
Index: b/security/apparmor/main.c
|
|
===================================================================
|
|
--- a/security/apparmor/main.c
|
|
+++ b/security/apparmor/main.c
|
|
@@ -748,7 +748,7 @@ int aa_clone(struct task_struct *child)
|
|
|
|
if (!aa_task_context(current))
|
|
return 0;
|
|
- child_cxt = aa_alloc_task_context(child);
|
|
+ child_cxt = aa_alloc_task_context();
|
|
if (!child_cxt)
|
|
return -ENOMEM;
|
|
|
|
@@ -984,18 +984,19 @@ void aa_release(struct task_struct *task
|
|
* list, another process could replace the profile under us,
|
|
* leaving us with a locked profile that is no longer attached
|
|
* to this task. So after locking the profile, we check that
|
|
- * the profile is still attached. If it is the profile lock
|
|
- * is sufficient to prevent the replacement race so we do not
|
|
- * lock the task. We avoid taking the task_lock here because
|
|
- * lock_dep will report an inconsistent {softirq-on-W} potential
|
|
- * irq_lock inversion, this specific case is safe but we would
|
|
- * still like to avoid the message.
|
|
+ * the profile is still attached. The profile lock is
|
|
+ * sufficient to prevent the replacement race so we do not lock
|
|
+ * the task.
|
|
*
|
|
- * If the task does not have a profile attached at this point,
|
|
- * we are safe.
|
|
+ * We also avoid taking the task_lock here because lock_dep
|
|
+ * would report a false {softirq-on-W} potential irq_lock
|
|
+ * inversion.
|
|
+ *
|
|
+ * If the task does not have a profile attached we are safe;
|
|
+ * nothing can race with us at this point.
|
|
*/
|
|
|
|
- repeat:
|
|
+repeat:
|
|
profile = aa_get_profile(task);
|
|
if (profile) {
|
|
lock_profile(profile);
|
|
@@ -1008,7 +1009,6 @@ void aa_release(struct task_struct *task
|
|
aa_change_task_context(task, NULL, NULL, 0);
|
|
unlock_profile(profile);
|
|
aa_put_profile(profile);
|
|
- aa_free_task_context_rcu(cxt);
|
|
}
|
|
}
|
|
|
|
@@ -1071,7 +1071,6 @@ static inline int do_change_hat(const ch
|
|
aa_change_task_context(current, new_cxt,
|
|
cxt->profile->null_profile, hat_magic);
|
|
}
|
|
- aa_free_task_context_rcu(cxt);
|
|
|
|
return error;
|
|
}
|
|
@@ -1100,7 +1099,7 @@ int aa_change_hat(const char *hat_name,
|
|
hat_magic, current->pid);
|
|
}
|
|
|
|
- new_cxt = aa_alloc_task_context(current);
|
|
+ new_cxt = aa_alloc_task_context();
|
|
if (!new_cxt)
|
|
return -ENOMEM;
|
|
|
|
@@ -1151,7 +1150,6 @@ int aa_change_hat(const char *hat_name,
|
|
/* Return from subprofile back to parent. */
|
|
aa_change_task_context(current, new_cxt,
|
|
profile->parent, 0);
|
|
- aa_free_task_context_rcu(cxt);
|
|
} else {
|
|
/*
|
|
* Change to another (sibling) profile, and
|
|
@@ -1194,15 +1192,6 @@ out:
|
|
return error;
|
|
}
|
|
|
|
-static inline void unlock_context_and_profile(struct aa_task_context *cxt,
|
|
- struct aa_profile *profile)
|
|
-{
|
|
- if (cxt)
|
|
- unlock_both_profiles(profile, cxt->profile);
|
|
- else
|
|
- unlock_profile(profile);
|
|
-}
|
|
-
|
|
/**
|
|
* aa_replace_profile - replace a task's profile
|
|
*/
|
|
@@ -1214,7 +1203,7 @@ struct aa_profile *aa_replace_profile(st
|
|
struct aa_profile *old_profile = NULL;
|
|
|
|
if (profile) {
|
|
- new_cxt = aa_alloc_task_context(task);
|
|
+ new_cxt = aa_alloc_task_context();
|
|
if (!new_cxt)
|
|
return ERR_PTR(-ENOMEM);
|
|
}
|
|
@@ -1222,20 +1211,19 @@ struct aa_profile *aa_replace_profile(st
|
|
cxt = lock_task_and_profiles(task, profile);
|
|
if (profile && profile->isstale) {
|
|
task_unlock(task);
|
|
- unlock_context_and_profile(cxt, profile);
|
|
+ unlock_both_profiles(profile, cxt ? cxt->profile : NULL);
|
|
aa_free_task_context(new_cxt);
|
|
return ERR_PTR(-ESTALE);
|
|
}
|
|
|
|
if (cxt) {
|
|
- aa_change_task_context(task, new_cxt, profile, cxt->hat_magic);
|
|
old_profile = aa_dup_profile(cxt->profile);
|
|
+ aa_change_task_context(task, new_cxt, profile, cxt->hat_magic);
|
|
} else
|
|
aa_change_task_context(task, new_cxt, profile, 0);
|
|
|
|
task_unlock(task);
|
|
- unlock_context_and_profile(cxt, profile);
|
|
- aa_free_task_context_rcu(cxt);
|
|
+ unlock_both_profiles(profile, old_profile);
|
|
|
|
return old_profile;
|
|
}
|
|
@@ -1243,6 +1231,7 @@ struct aa_profile *aa_replace_profile(st
|
|
void free_aa_task_context_rcu_callback(struct rcu_head *head)
|
|
{
|
|
struct aa_task_context *cxt;
|
|
+
|
|
cxt = container_of(head, struct aa_task_context, rcu);
|
|
aa_free_task_context(cxt);
|
|
}
|
|
@@ -1261,22 +1250,21 @@ void free_aa_task_context_rcu_callback(s
|
|
struct aa_task_context *
|
|
lock_task_and_profiles(struct task_struct *task, struct aa_profile *profile)
|
|
{
|
|
- struct aa_task_context *cxt, *check_cxt;
|
|
+ struct aa_task_context *cxt;
|
|
+ struct aa_profile *old_profile = NULL;
|
|
|
|
rcu_read_lock();
|
|
repeat:
|
|
cxt = aa_task_context(task);
|
|
if (cxt)
|
|
- lock_both_profiles(profile, cxt->profile);
|
|
- else
|
|
- lock_profile(profile);
|
|
+ old_profile = cxt->profile;
|
|
+ lock_both_profiles(profile, old_profile);
|
|
task_lock(task);
|
|
- check_cxt = aa_task_context(task);
|
|
|
|
/* check for race with profile transition, replacement or removal */
|
|
- if (unlikely(check_cxt != cxt)) {
|
|
+ if (unlikely(cxt != aa_task_context(task))) {
|
|
task_unlock(task);
|
|
- unlock_context_and_profile(cxt, profile);
|
|
+ unlock_both_profiles(profile, old_profile);
|
|
goto repeat;
|
|
}
|
|
rcu_read_unlock();
|
|
Index: b/security/apparmor/module_interface.c
|
|
===================================================================
|
|
--- a/security/apparmor/module_interface.c
|
|
+++ b/security/apparmor/module_interface.c
|
|
@@ -459,9 +459,17 @@ ssize_t aa_file_prof_replace(void *udata
|
|
if (!old_profile)
|
|
goto out;
|
|
|
|
+ /*
|
|
+ * FIXME: this loop is confusing. Can't we simply allocate the new
|
|
+ * task context under the profile locks after checking that we
|
|
+ * actually need it?
|
|
+ */
|
|
do {
|
|
- new_cxt = aa_alloc_task_context(NULL);
|
|
- /* FIXME: what do we do when allocation fails */
|
|
+ new_cxt = aa_alloc_task_context();
|
|
+ /*
|
|
+ * FIXME: what do we do when allocation fails --
|
|
+ * I guess allocate in a way that cannot fail.
|
|
+ */
|
|
if (!new_cxt)
|
|
break;
|
|
|
|
@@ -474,13 +482,12 @@ ssize_t aa_file_prof_replace(void *udata
|
|
*/
|
|
lock_both_profiles(old_profile, new_profile);
|
|
if (!list_empty(&old_profile->task_contexts)) {
|
|
- struct aa_task_context *cxt =
|
|
+ struct task_struct *task =
|
|
list_entry(old_profile->task_contexts.next,
|
|
- struct aa_task_context, list);
|
|
- task_lock(cxt->task);
|
|
- task_replace(cxt->task, new_cxt, new_profile);
|
|
- task_unlock(cxt->task);
|
|
- aa_free_task_context_rcu(cxt);
|
|
+ struct aa_task_context, list)->task;
|
|
+ task_lock(task);
|
|
+ task_replace(task, new_cxt, new_profile);
|
|
+ task_unlock(task);
|
|
new_cxt = NULL;
|
|
}
|
|
unlock_both_profiles(old_profile, new_profile);
|
|
@@ -520,13 +527,12 @@ ssize_t aa_file_prof_remove(const char *
|
|
|
|
lock_profile(profile);
|
|
while (!list_empty(&profile->task_contexts)) {
|
|
- struct aa_task_context *cxt =
|
|
+ struct task_struct *task =
|
|
list_entry(profile->task_contexts.next,
|
|
- struct aa_task_context, list);
|
|
- task_lock(cxt->task);
|
|
- aa_change_task_context(cxt->task, NULL, NULL, 0);
|
|
- task_unlock(cxt->task);
|
|
- aa_free_task_context_rcu(cxt);
|
|
+ struct aa_task_context, list)->task;
|
|
+ task_lock(task);
|
|
+ aa_change_task_context(task, NULL, NULL, 0);
|
|
+ task_unlock(task);
|
|
}
|
|
unlock_profile(profile);
|
|
mutex_unlock(&aa_interface_lock);
|