1
Index: b/security/apparmor/apparmor.h
2
===================================================================
3
--- a/security/apparmor/apparmor.h
4
+++ b/security/apparmor/apparmor.h
5
@@ -128,9 +128,6 @@ struct aa_profile {
6
extern struct list_head profile_list;
7
extern rwlock_t profile_list_lock;
9
-extern struct list_head task_context_list;
10
-extern rwlock_t task_context_list_lock;
13
* struct aa_task_context - primary label for confined tasks
14
* @profile: the current profile
15
@@ -155,9 +152,6 @@ static inline struct aa_task_context *aa
16
return (struct aa_task_context *)task->security;
19
-/* Lock protecting access to 'struct aa_task_context' accesses */
20
-extern spinlock_t cxt_lock;
22
extern struct aa_profile *null_complain_profile;
24
/* aa_audit - AppArmor auditing structure
25
@@ -242,9 +236,6 @@ extern struct aa_profile *__aa_find_prof
28
extern void aa_profilelist_release(void);
29
-extern void aa_task_context_list_add(struct aa_task_context *);
30
-extern void aa_task_context_list_remove(struct aa_task_context *);
31
-extern void aa_task_context_list_release(void);
33
/* module_interface.c */
34
extern ssize_t aa_file_prof_add(void *, size_t);
35
Index: b/security/apparmor/inline.h
36
===================================================================
37
--- a/security/apparmor/inline.h
38
+++ b/security/apparmor/inline.h
39
@@ -42,7 +42,7 @@ static inline struct aa_profile *aa_get_
41
cxt = aa_task_context(task);
43
- profile = (struct aa_profile *)rcu_dereference(cxt->profile);
44
+ profile = rcu_dereference(cxt->profile);
45
aa_dup_profile(profile);
48
@@ -79,41 +79,22 @@ static inline void aa_change_profile(str
49
aa_put_profile(old_profile);
53
- * alloc_aa_task_context - allocate a new aa_task_context
54
- * @task: task struct
56
- * Allocate a new aa_task_context including a backpointer to it's referring
59
-static inline struct aa_task_context *alloc_aa_task_context(struct task_struct *task)
60
+static inline struct aa_task_context *
61
+aa_alloc_task_context(struct task_struct *task)
63
struct aa_task_context *cxt;
65
cxt = kzalloc(sizeof(struct aa_task_context), GFP_KERNEL);
69
- /* back pointer to task */
72
- /* any readers of the list must make sure that they can handle
73
- * case where cxt->profile is not yet set (null)
75
- aa_task_context_list_add(cxt);
77
+ INIT_LIST_HEAD(&cxt->list);
86
- * free_aa_task_context - Free a aa_task_context previously allocated by
87
- * alloc_aa_task_context
88
- * @cxt: aa_task_context
90
-static inline void free_aa_task_context(struct aa_task_context *cxt)
91
+static inline void aa_free_task_context(struct aa_task_context *cxt)
93
- aa_task_context_list_remove(cxt);
97
Index: b/security/apparmor/list.c
98
===================================================================
99
--- a/security/apparmor/list.c
100
+++ b/security/apparmor/list.c
102
LIST_HEAD(profile_list);
103
rwlock_t profile_list_lock = RW_LOCK_UNLOCKED;
105
-/* list of all task_contexts and lock */
106
-LIST_HEAD(task_context_list);
107
-rwlock_t task_context_list_lock = RW_LOCK_UNLOCKED;
110
* __aa_find_profile - look up a profile on the profile list
111
* @name: name of profile to find
112
@@ -56,58 +52,6 @@ void aa_profilelist_release(void)
113
write_unlock(&profile_list_lock);
117
- * aa_task_context_list_add - Add aa_task_context to task_context_list
118
- * @cxt: new aa_task_context
120
-void aa_task_context_list_add(struct aa_task_context *cxt)
122
- unsigned long flags;
125
- AA_INFO("%s: bad aa_task_context\n", __FUNCTION__);
129
- write_lock_irqsave(&task_context_list_lock, flags);
130
- /* new aa_task_contexts must be added to the end of the list due to a
131
- * subtle interaction between fork and profile replacement.
133
- list_add_tail(&cxt->list, &task_context_list);
134
- write_unlock_irqrestore(&task_context_list_lock, flags);
138
- * aa_task_context_list_remove - Remove aa_task_context from task_context_list
139
- * @cxt: aa_task_context to be removed
141
-void aa_task_context_list_remove(struct aa_task_context *cxt)
143
- unsigned long flags;
146
- write_lock_irqsave(&task_context_list_lock, flags);
147
- list_del_init(&cxt->list);
148
- write_unlock_irqrestore(&task_context_list_lock, flags);
153
- * aa_task_context_list_release - Remove all aa_task_contexts from
154
- * task_context_list
156
-void aa_task_context_list_release(void)
158
- struct aa_task_context *node, *tmp;
159
- unsigned long flags;
161
- write_lock_irqsave(&task_context_list_lock, flags);
162
- list_for_each_entry_safe(node, tmp, &task_context_list, list) {
163
- list_del_init(&node->list);
165
- write_unlock_irqrestore(&task_context_list_lock, flags);
168
/* seq_file helper routines
169
* Used by apparmorfs.c to iterate over profile_list
171
Index: b/security/apparmor/lsm.c
172
===================================================================
173
--- a/security/apparmor/lsm.c
174
+++ b/security/apparmor/lsm.c
176
#include "apparmor.h"
179
-/* struct aa_task_context write update lock (read side is RCU). */
180
-spinlock_t cxt_lock = SPIN_LOCK_UNLOCKED;
182
/* Flag values, also controllable via apparmorfs/control.
183
* We explicitly do not allow these to be modifiable when exported via
184
* /sys/modules/parameters, as we want to do additional mediation and
185
@@ -749,49 +746,68 @@ createfs_out:
187
static void __exit apparmor_exit(void)
189
- struct aa_task_context *cxt;
190
- unsigned long flags;
191
+ LIST_HEAD(task_contexts_bucket);
193
- /* Remove profiles from the global profile list.
194
- * This is just for tidyness as there is no way to reference this
195
- * list once the AppArmor lsm hooks are detached (below)
197
- aa_profilelist_release();
198
+ /* Remove and release all the profiles on the profile list. */
199
+ write_lock(&profile_list_lock);
200
+ while (!list_empty(&profile_list)) {
201
+ struct aa_profile *profile =
202
+ list_entry(profile_list.next, struct aa_profile, list);
204
+ /* Remove the profile from each task context it is on. */
205
+ lock_profile(profile);
206
+ while (!list_empty(&profile->task_contexts)) {
207
+ struct aa_task_context *cxt =
208
+ list_entry(profile->task_contexts.next,
209
+ struct aa_task_context, list);
212
+ * Detach the task context from the task. Protect
213
+ * from races by taking the task lock here.
215
+ task_lock(cxt->task);
216
+ rcu_assign_pointer(cxt->task->security, NULL);
217
+ task_unlock(cxt->task);
218
+ aa_change_profile(cxt, NULL, 0);
220
- /* Remove profiles from profiled tasks
221
- * If this is not done, if module is reloaded after being removed,
222
- * old profiles (still refcounted in memory) will become 'magically'
226
+ * Defer release: the task context may still have
229
+ list_move(&cxt->list, &task_contexts_bucket);
231
+ unlock_profile(profile);
234
- * FIXME: We have a lock inversion here (cp. aa_file_prof_repl,
235
- * aa_file_prof_remove).
237
- spin_lock_irqsave(&cxt_lock, flags);
238
- read_lock(&task_context_list_lock);
239
- list_for_each_entry(cxt, &task_context_list, list) {
241
- aa_change_profile(cxt, NULL, 0);
242
+ /* Release the profile itself. */
243
+ list_del_init(&profile->list);
244
+ aa_put_profile(profile);
246
- read_unlock(&task_context_list_lock);
247
- spin_unlock_irqrestore(&cxt_lock, flags);
249
- /* Free up list of profile aa_task_context */
250
- aa_task_context_list_release();
251
+ write_unlock(&profile_list_lock);
253
free_null_complain_profile();
256
+ * Delay for an rcu cycle to make sure that all active task
257
+ * context readers have finished, and all profiles have been
258
+ * freed by their rcu callbacks.
262
+ /* Now we can safely free all remaining task contexts. */
263
+ while (!list_empty(&task_contexts_bucket)) {
264
+ struct aa_task_context *cxt =
265
+ list_entry(task_contexts_bucket.next,
266
+ struct aa_task_context, list);
268
+ list_del(&cxt->list);
272
destroy_apparmorfs();
274
if (unregister_security(&apparmor_ops))
275
AA_WARN("Unable to properly unregister AppArmor\n");
277
- /* delay for an rcu cycle to make ensure that profiles pending
278
- * destruction in the rcu callback are freed.
282
AA_INFO("AppArmor protection removed\n");
283
aa_audit_message(NULL, GFP_KERNEL, 0,
284
"AppArmor protection removed\n");
285
Index: b/security/apparmor/main.c
286
===================================================================
287
--- a/security/apparmor/main.c
288
+++ b/security/apparmor/main.c
289
@@ -717,19 +717,9 @@ int aa_link(struct aa_profile *profile,
290
*******************************/
293
- * aa_fork - create a new aa_task_context
294
- * @task: new process
296
- * Create a new aa_task_context for newly created process @task if it's parent
297
- * is already confined. Otherwise a aa_task_context will be lazily allocated
298
- * will get one with NULL values. Return 0 on sucess.
299
- * for the child if it subsequently execs (in aa_register).
300
- * Return 0 on sucess.
302
- * The cxt_lock is used to maintain consistency against profile
303
- * replacement/removal.
304
+ * aa_fork - initialize the task context for a new task
305
+ * @task: task that is being created
308
int aa_fork(struct task_struct *task)
310
struct aa_task_context *cxt = aa_task_context(current);
311
@@ -738,9 +728,7 @@ int aa_fork(struct task_struct *task)
312
AA_DEBUG("%s\n", __FUNCTION__);
314
if (cxt && cxt->profile) {
315
- unsigned long flags;
317
- newcxt = alloc_aa_task_context(task);
318
+ newcxt = aa_alloc_task_context(task);
320
/* FIXME: The alloc above is a blocking operation, so
321
* cxt->profile may have vanished by now.
322
@@ -758,9 +746,7 @@ int aa_fork(struct task_struct *task)
323
* race with profile replacement or removal here, and
324
* he new task would end up with an obsolete profile.
326
- spin_lock_irqsave(&cxt_lock, flags);
327
aa_change_profile(newcxt, cxt->profile, cxt->hat_magic);
328
- spin_unlock_irqrestore(&cxt_lock, flags);
330
if (APPARMOR_COMPLAIN(cxt) &&
331
cxt->profile == null_complain_profile)
332
@@ -910,7 +896,6 @@ repeat:
333
/* Apply the new profile, or switch to unconfined if NULL. */
334
if (!IS_ERR(newprofile)) {
335
struct aa_task_context *cxt, *lazy_cxt = NULL;
336
- unsigned long flags;
338
/* grab a lock - this is to guarentee consistency against
339
* other writers of aa_task_context (replacement/removal)
340
@@ -936,7 +921,7 @@ repeat:
343
if (!profile && !(cxt = aa_task_context(current))) {
344
- lazy_cxt = alloc_aa_task_context(current);
345
+ lazy_cxt = aa_alloc_task_context(current);
347
AA_ERROR("%s: Failed to allocate aa_task_context\n",
349
@@ -945,13 +930,11 @@ repeat:
353
- spin_lock_irqsave(&cxt_lock, flags);
355
cxt = aa_task_context(current);
358
/* raced by setprofile - created cxt */
359
- free_aa_task_context(lazy_cxt);
360
+ aa_free_task_context(lazy_cxt);
363
/* Not rcu used to get the write barrier
364
@@ -977,7 +960,6 @@ repeat:
365
/* Race, profile was removed, not replaced.
366
* Redo with error checking
368
- spin_unlock_irqrestore(&cxt_lock, flags);
372
@@ -1005,8 +987,6 @@ repeat:
373
LOG_HINT(newprofile, GFP_ATOMIC, HINT_CHGPROF,
377
- spin_unlock_irqrestore(&cxt_lock, flags);
381
@@ -1019,28 +999,51 @@ cleanup:
385
- * aa_release - release the task's aa_task_context
386
+ * aa_release - release a task context
387
* @task: task being released
389
* This is called after a task has exited and the parent has reaped it.
390
- * @task->security blob is freed.
392
- * This is the one case where we don't need to hold the cxt_lock before
393
- * removing a profile from a aa_task_context. Once the aa_task_context has
394
- * been removed from the aa_task_context_list, we are no longer racing other
395
- * writers. There may still be other readers so we must still use
396
- * aa_change_profile to put the aa_task_context's reference safely.
398
void aa_release(struct task_struct *task)
400
- struct aa_task_context *cxt = aa_task_context(task);
402
- task->security = NULL;
403
+ struct aa_task_context *cxt = NULL;
404
+ struct aa_profile *profile;
406
- aa_task_context_list_remove(cxt);
407
- aa_change_profile(cxt, NULL, 0);
409
+ * While the task context is still on a profile's task context
410
+ * list, another process could replace the profile under us,
411
+ * leaving us with a locked profile that is no longer attached
412
+ * to this task. So after locking the profile, we lock the task
413
+ * to make sure that the profile is still attached. (We must
414
+ * lock the profile first to avoid lock inversion.)
416
+ * If the task does not have a profile attached at this point,
421
+ profile = aa_get_profile(task);
423
+ lock_profile(profile);
425
+ cxt = aa_task_context(task);
426
+ if (unlikely(profile != cxt->profile)) {
428
+ unlock_profile(profile);
429
+ aa_put_profile(profile);
433
+ * There may still be other profile readers, so we must
434
+ * put the profile reference safely.
436
+ aa_change_profile(cxt, NULL, 0);
438
+ list_del(&cxt->list);
439
+ unlock_profile(profile);
440
+ aa_put_profile(profile);
442
+ task->security = NULL;
446
Index: b/security/apparmor/procattr.c
447
===================================================================
448
--- a/security/apparmor/procattr.c
449
+++ b/security/apparmor/procattr.c
450
@@ -82,7 +82,6 @@ int aa_setprocattr_changehat(char *hatin
451
char *token = NULL, *hat, *smagic, *tmp;
453
int rc, len, consumed;
454
- unsigned long flags;
456
AA_DEBUG("%s: %p %zd\n", __FUNCTION__, hatinfo, infosize);
458
@@ -158,9 +157,7 @@ int aa_setprocattr_changehat(char *hatin
459
AA_DEBUG("%s: Magic 0x%x Hat '%s'\n",
460
__FUNCTION__, magic, hat ? hat : NULL);
462
- spin_lock_irqsave(&cxt_lock, flags);
463
error = aa_change_hat(hat, magic);
464
- spin_unlock_irqrestore(&cxt_lock, flags);
468
@@ -178,7 +175,6 @@ int aa_setprocattr_setprofile(struct tas
469
struct aa_profile *profile = NULL;
470
struct aa_task_context *cxt;
472
- unsigned long flags;
474
AA_DEBUG("%s: current %s(%d)\n",
475
__FUNCTION__, current->comm, current->pid);
476
@@ -221,8 +217,6 @@ int aa_setprocattr_setprofile(struct tas
480
- spin_lock_irqsave(&cxt_lock, flags);
482
cxt = aa_task_context(task);
484
/* switch to unconstrained */
485
@@ -249,10 +243,7 @@ int aa_setprocattr_setprofile(struct tas
486
AA_WARN("%s: task %s(%d) has no aa_task_context\n",
487
__FUNCTION__, task->comm, task->pid);
489
- /* unlock so we can safely GFP_KERNEL */
490
- spin_unlock_irqrestore(&cxt_lock, flags);
492
- cxt = alloc_aa_task_context(task);
493
+ cxt = aa_alloc_task_context(task);
495
AA_WARN("%s: Unable to allocate "
496
"aa_task_context for task %s(%d). "
497
@@ -267,11 +258,10 @@ int aa_setprocattr_setprofile(struct tas
501
- spin_lock_irqsave(&cxt_lock, flags);
502
if (!aa_task_context(task)) {
503
task->security = cxt;
505
- free_aa_task_context(cxt);
506
+ aa_free_task_context(cxt);
507
cxt = aa_task_context(task);
510
@@ -287,7 +277,6 @@ int aa_setprocattr_setprofile(struct tas
513
/* Race, profile was removed. */
514
- spin_unlock_irqrestore(&cxt_lock, flags);
518
@@ -312,8 +301,6 @@ int aa_setprocattr_setprofile(struct tas
519
aa_put_profile(profile);
522
- spin_unlock_irqrestore(&cxt_lock, flags);
527
Index: b/security/apparmor/module_interface.c
528
===================================================================
529
--- a/security/apparmor/module_interface.c
530
+++ b/security/apparmor/module_interface.c
531
@@ -460,29 +460,24 @@ ssize_t aa_file_prof_repl(void *udata, s
532
old_profile = __aa_find_profile(new_profile->name, &profile_list);
534
struct aa_task_context *cxt;
535
- unsigned long flags;
537
- list_del_init(&old_profile->list);
539
lock_profile(old_profile);
540
old_profile->isstale = 1;
541
- unlock_profile(old_profile);
544
- * Find all tasks using the old profile and replace the old
545
- * profile with the new.
547
- read_lock_irqsave(&task_context_list_lock, flags);
548
- list_for_each_entry(cxt, &task_context_list, list) {
549
- spin_lock_irqsave(&cxt_lock, flags);
551
- if (cxt->profile &&
552
- cxt->profile->parent == old_profile)
553
- task_replace(cxt, new_profile);
555
- spin_unlock_irqrestore(&cxt_lock, flags);
556
+ list_for_each_entry(cxt, &old_profile->task_contexts, list) {
558
+ * Protect from racing with aa_release by taking
559
+ * the task lock here.
561
+ task_lock(cxt->task);
562
+ task_replace(cxt, new_profile);
563
+ task_unlock(cxt->task);
565
- read_unlock_irqrestore(&task_context_list_lock, flags);
566
+ list_splice_init(&old_profile->task_contexts,
567
+ &new_profile->task_contexts);
568
+ unlock_profile(old_profile);
570
+ list_del_init(&old_profile->list);
571
aa_put_profile(old_profile);
573
list_add(&new_profile->list, &profile_list);
574
@@ -502,8 +497,6 @@ ssize_t aa_file_prof_repl(void *udata, s
575
ssize_t aa_file_prof_remove(const char *name, size_t size)
577
struct aa_profile *profile;
578
- struct aa_task_context *cxt;
579
- unsigned long flags;
581
write_lock(&profile_list_lock);
582
profile = __aa_find_profile(name, &profile_list);
583
@@ -512,24 +505,22 @@ ssize_t aa_file_prof_remove(const char *
587
- list_del_init(&profile->list);
589
lock_profile(profile);
590
profile->isstale = 1;
591
- unlock_profile(profile);
593
- read_lock_irqsave(&task_context_list_lock, flags);
594
- list_for_each_entry(cxt, &task_context_list, list) {
595
- if (cxt->profile && cxt->profile->parent == profile) {
596
- spin_lock(&cxt_lock);
597
- aa_change_profile(cxt, NULL, 0);
598
- spin_unlock(&cxt_lock);
600
+ while (!list_empty(&profile->task_contexts)) {
601
+ struct aa_task_context *cxt =
602
+ list_entry(profile->task_contexts.next,
603
+ struct aa_task_context, list);
604
+ task_lock(cxt->task);
605
+ aa_change_profile(cxt, NULL, 0);
606
+ task_unlock(cxt->task);
607
+ list_del_init(&cxt->list);
609
- read_unlock_irqrestore(&task_context_list_lock, flags);
610
+ unlock_profile(profile);
612
- aa_put_profile(profile);
613
+ list_del_init(&profile->list);
614
write_unlock(&profile_list_lock);
615
+ aa_put_profile(profile);
added
removed
kernel-patches/for-mainline/rework-locking.diff
