← Back to branch summary

~ubuntu-branches/ubuntu/wily/apparmor/wily

« back to all changes in this revision

Viewing changes to kernel-patches/for-mainline/aa_switch.diff

  • Committer: Bazaar Package Importer
  • Author(s): Kees Cook
  • Date: 2011-04-27 10:38:07 UTC
  • mfrom: (5.1.118 natty)
  • Revision ID: james.westby@ubuntu.com-20110427103807-ym3rhwys6o84ith0
Tags: 2.6.1-2
debian/copyright: clarify for some full organization names.

Show diffs side-by-side

added added

removed removed

Lines of Context:
kernel-patches/for-mainline/aa_switch.diff
1
 
Rename aa_switch() to aa_switch_to_profile(), and replace
2
 
aa_switch_unconfined(task) with aa_switch_to_profile(task, NULL, 0):
3
 
this is easier to understand. Pass the hat magic to switch to to
4
 
aa_switch_to_profile as well: we want to make it more explicit
5
 
which profile switches go together with which hat changes!
6
 
 
7
 
This patch adds a FIXME note about a race in aa_fork().
8
 
 
9
 
Also a FIXME: a_register() is much too long and needs to be broken
10
 
up into sensible pieces.
11
 
 
12
 
Index: b/security/apparmor/inline.h
13
 
===================================================================
14
 
--- a/security/apparmor/inline.h
15
 
+++ b/security/apparmor/inline.h
16
 
@@ -51,42 +51,28 @@ static inline struct aa_profile *aa_get_
17
 
 }
18
 
 
19
 
 /**
20
 
- * aa_switch - change aa_task_context to use a new profile
21
 
+ * aa_switch_to_profile - change aa_task_context to use a new profile
22
 
  * @cxt: aa_task_context to switch the active profile on
23
 
- * @newactive: new active profile
24
 
+ * @newactive: new active profile (NULL for unconfined)
25
 
+ * @hat_magic: hat value to switch to (0 for no hat)
26
 
  *
27
 
- * aa_switch handles the changing of a aa_task_context's active profile.  The
28
 
- * cxt_lock must be held to ensure consistency against other writers.
29
 
- * Some write paths (ex. aa_register) require cxt->active not to change
30
 
- * over several operations, so the calling function is responsible
31
 
- * for grabing the cxt_lock to meet its consistency constraints before
32
 
- * calling aa_switch
33
 
- */
34
 
-static inline void aa_switch(struct aa_task_context *cxt,
35
 
-                            struct aa_profile *newactive)
36
 
+ * aa_switch_to_profile handles the changing of a aa_task_context's active
37
 
+ * profile.  The cxt_lock must be held to ensure consistency against
38
 
+ * other writers.  Some write paths (ex. aa_register) require
39
 
+ * cxt->active not to change over several operations, so the calling
40
 
+ * function is responsible for grabing the cxt_lock to meet its
41
 
+ * consistency constraints before calling aa_switch_to_profile
42
 
+ */
43
 
+static inline void aa_switch_to_profile(struct aa_task_context *cxt,
44
 
+                                       struct aa_profile *newactive,
45
 
+                                       u32 hat_magic)
46
 
 {
47
 
-       struct aa_profile *oldactive = cxt->active;
48
 
+       struct aa_profile *old = cxt->active;
49
 
 
50
 
-       /* noop if NULL */
51
 
-       rcu_assign_pointer(cxt->active, aa_dup_profile(newactive));
52
 
        cxt->caps_logged = CAP_EMPTY_SET;
53
 
-       put_aa_profile(oldactive);
54
 
-}
55
 
-
56
 
-/**
57
 
- * aa_switch_unconfined - change aa_task_context to be unconfined (no profile)
58
 
- * @cxt: aa_task_context to switch
59
 
- *
60
 
- * aa_switch_unconfined handles the removal of a aa_task_context's active
61
 
- * profile. The cxt_lock must be held to ensure consistency against other
62
 
- * writers. Like aa_switch the cxt_lock is used to maintain consistency.
63
 
- */
64
 
-static inline void aa_switch_unconfined(struct aa_task_context *cxt)
65
 
-{
66
 
-       aa_switch(cxt, NULL);
67
 
-
68
 
-       /* reset magic in case we were in a subhat before */
69
 
-       cxt->hat_magic = 0;
70
 
+       cxt->hat_magic = hat_magic;
71
 
+       rcu_assign_pointer(cxt->active, aa_dup_profile(newactive));
72
 
+       put_aa_profile(old);
73
 
 }
74
 
 
75
 
 /**
76
 
Index: b/security/apparmor/main.c
77
 
===================================================================
78
 
--- a/security/apparmor/main.c
79
 
+++ b/security/apparmor/main.c
80
 
@@ -749,16 +749,24 @@ int aa_fork(struct task_struct *p)
81
 
 
82
 
                newcxt = alloc_aa_task_context(p);
83
 
 
84
 
+               /* FIXME: The alloc above is a blocking operation, so
85
 
+                *        cxt->active may have vanished by now.
86
 
+                *        We really need to do the full stale checking
87
 
+                *        thing here, too.
88
 
+                */
89
 
+
90
 
                if (!newcxt)
91
 
                        return -ENOMEM;
92
 
 
93
 
-               /* Use locking here instead of getting the reference
94
 
+               /*
95
 
+                * Use locking here instead of getting the reference
96
 
                 * because we need both the old reference and the
97
 
-                * new reference to be consistent.
98
 
+                * new reference to be consistent: otherwise, we could
99
 
+                * race with profile replacement or removal here, and
100
 
+                * he new task would end up with an obsolete profile.
101
 
                 */
102
 
                spin_lock_irqsave(&cxt_lock, flags);
103
 
-               aa_switch(newcxt, cxt->active);
104
 
-               newcxt->hat_magic = cxt->hat_magic;
105
 
+               aa_switch_to_profile(newcxt, cxt->active, cxt->hat_magic);
106
 
                spin_unlock_irqrestore(&cxt_lock, flags);
107
 
 
108
 
                if (APPARMOR_COMPLAIN(cxt) &&
109
 
@@ -1033,7 +1041,7 @@ apply_profile:
110
 
                                ((unsigned long)bprm->security | bprm_flags);
111
 
                }
112
 
 
113
 
-               aa_switch(cxt, newprofile);
114
 
+               aa_switch_to_profile(cxt, newprofile, 0);
115
 
                put_aa_profile(newprofile);
116
 
 
117
 
                if (complain && newprofile == null_complain_profile)
118
 
@@ -1063,8 +1071,8 @@ out:
119
 
  * This is the one case where we don't need to hold the cxt_lock before
120
 
  * removing a profile from a aa_task_context.  Once the aa_task_context has
121
 
  * been removed from the aa_task_context_list, we are no longer racing other
122
 
- * writers. There may still be other readers so we must still use aa_switch
123
 
- * to put the aa_task_context's reference safely.
124
 
+ * writers. There may still be other readers so we must still use
125
 
+ * aa_switch_to_profile to put the aa_task_context's reference safely.
126
 
  */
127
 
 void aa_release(struct task_struct *p)
128
 
 {
129
 
@@ -1073,7 +1081,7 @@ void aa_release(struct task_struct *p)
130
 
                p->security = NULL;
131
 
 
132
 
                aa_task_context_list_remove(cxt);
133
 
-               aa_switch_unconfined(cxt);
134
 
+               aa_switch_to_profile(cxt, NULL, 0);
135
 
 
136
 
                kfree(cxt);
137
 
        }
138
 
@@ -1090,7 +1098,9 @@ void aa_release(struct task_struct *p)
139
 
  *
140
 
  * Switch to a new hat.  Return %0 on success, error otherwise.
141
 
  */
142
 
-static inline int do_change_hat(const char *hat_name, struct aa_task_context *cxt)
143
 
+static inline int do_change_hat(const char *hat_name,
144
 
+                               struct aa_task_context *cxt,
145
 
+                               u32 hat_magic)
146
 
 {
147
 
        struct aa_profile *sub;
148
 
        int error = 0;
149
 
@@ -1099,7 +1109,7 @@ static inline int do_change_hat(const ch
150
 
 
151
 
        if (sub) {
152
 
                /* change hat */
153
 
-               aa_switch(cxt, sub);
154
 
+               aa_switch_to_profile(cxt, sub, hat_magic);
155
 
                put_aa_profile(sub);
156
 
        } else {
157
 
                /* There is no such subprofile change to a NULL profile.
158
 
@@ -1108,7 +1118,7 @@ static inline int do_change_hat(const ch
159
 
                 * This feature is used by changehat_apache.
160
 
                 *
161
 
                 * N.B from the null-profile the task can still changehat back
162
 
-                * out to the parent profile (assuming magic != NULL)
163
 
+                * out to the parent profile (assuming magic != 0)
164
 
                 */
165
 
                if (APPARMOR_COMPLAIN(cxt)) {
166
 
                        LOG_HINT(cxt->active, GFP_ATOMIC, HINT_UNKNOWN_HAT,
167
 
@@ -1129,7 +1139,7 @@ static inline int do_change_hat(const ch
168
 
                                 cxt->active->name);
169
 
                        error = -EACCES;
170
 
                }
171
 
-               aa_switch(cxt, cxt->active->null_profile);
172
 
+               aa_switch_to_profile(cxt, cxt->active->null_profile, hat_magic);
173
 
        }
174
 
 
175
 
        return error;
176
 
@@ -1196,8 +1206,7 @@ int aa_change_hat(const char *hat_name, 
177
 
                         * (or null-profile, if the hat doesn't exist) until
178
 
                         * the task terminates
179
 
                         */
180
 
-                       cxt->hat_magic = hat_magic;
181
 
-                       error = do_change_hat(hat_name, cxt);
182
 
+                       error = do_change_hat(hat_name, cxt, hat_magic);
183
 
                } else {
184
 
                        /* Got here via changehat(NULL, magic)
185
 
                         *
186
 
@@ -1212,21 +1221,21 @@ int aa_change_hat(const char *hat_name, 
187
 
                 * Handle special casing of NULL magic which confines task
188
 
                 * to subprofile and prohibits further changehats
189
 
                 */
190
 
-               if (hat_magic == cxt->hat_magic && cxt->hat_magic) {
191
 
+               if (hat_magic && hat_magic == cxt->hat_magic) {
192
 
                        if (!hat_name) {
193
 
                                /*
194
 
                                 * Got here via changehat(NULL, magic)
195
 
                                 * Return from subprofile, back to parent
196
 
                                 */
197
 
-                               aa_switch(cxt, cxt->active->parent);
198
 
-
199
 
-                               /* Reset hat_magic to zero.
200
 
-                                * New value will be passed on next changehat
201
 
-                                */
202
 
-                               cxt->hat_magic = 0;
203
 
+                               aa_switch_to_profile(cxt, cxt->active->parent,
204
 
+                                                    0);
205
 
                        } else {
206
 
-                               /* change to another (sibling) profile */
207
 
-                               error = do_change_hat(hat_name, cxt);
208
 
+                               /*
209
 
+                                * Change to another (sibling) profile, and
210
 
+                                * stick with the same hat_magic.
211
 
+                                */
212
 
+                               error = do_change_hat(hat_name, cxt,
213
 
+                                                     cxt->hat_magic);
214
 
                        }
215
 
                } else if (cxt->hat_magic) {
216
 
                        AA_ERROR("KILLING process %s(%d) "
217
 
@@ -1240,7 +1249,7 @@ int aa_change_hat(const char *hat_name, 
218
 
 
219
 
                        /* terminate current process */
220
 
                        (void)send_sig_info(SIGKILL, NULL, current);
221
 
-               } else {        /* cxt->hat_magic == NULL */
222
 
+               } else {        /* cxt->hat_magic == 0 */
223
 
                        AA_ERROR("KILLING process %s(%d) "
224
 
                                 "Task was confined to current subprofile "
225
 
                                 "(profile %s active %s)\n",
226
 
Index: b/security/apparmor/lsm.c
227
 
===================================================================
228
 
--- a/security/apparmor/lsm.c
229
 
+++ b/security/apparmor/lsm.c
230
 
@@ -732,7 +732,7 @@ static int apparmor_exit_removeall_iter(
231
 
                         BASE_PROFILE(cxt->active)->name,
232
 
                         BASE_PROFILE(cxt->active),
233
 
                         cxt->active->name, cxt->active);
234
 
-               aa_switch_unconfined(cxt);
235
 
+               aa_switch_to_profile(cxt, NULL, 0);
236
 
        }
237
 
 
238
 
        return 0;
239
 
Index: b/security/apparmor/procattr.c
240
 
===================================================================
241
 
--- a/security/apparmor/procattr.c
242
 
+++ b/security/apparmor/procattr.c
243
 
@@ -235,7 +235,7 @@ int aa_setprocattr_setprofile(struct tas
244
 
                                BASE_PROFILE(cxt->active)->name,
245
 
                                cxt->active->name);
246
 
 
247
 
-                       aa_switch_unconfined(cxt);
248
 
+                       aa_switch_to_profile(cxt, NULL, 0);
249
 
                } else {
250
 
                        AA_WARN("%s: task %s(%d) "
251
 
                                "is already unconstrained\n",
252
 
@@ -308,17 +308,8 @@ int aa_setprocattr_setprofile(struct tas
253
 
                        cxt->active ? cxt->active->name : "unconstrained",
254
 
                        name);
255
 
 
256
 
-               aa_switch(cxt, profile);
257
 
-
258
 
-               put_aa_profile(profile); /* drop ref we obtained above
259
 
-                                        * from aa_profilelist_find
260
 
-                                        */
261
 
-
262
 
-               /* Reset magic in case we were in a subhat before
263
 
-                * This is the only case where we zero the magic after
264
 
-                * calling aa_switch
265
 
-                */
266
 
-               cxt->hat_magic = 0;
267
 
+               aa_switch_to_profile(cxt, profile, 0);
268
 
+               put_aa_profile(profile);
269
 
        }
270
 
 
271
 
        spin_unlock_irqrestore(&cxt_lock, flags);
272
 
Index: b/security/apparmor/module_interface.c
273
 
===================================================================
274
 
--- a/security/apparmor/module_interface.c
275
 
+++ b/security/apparmor/module_interface.c
276
 
@@ -54,7 +54,7 @@ static inline void task_remove(struct aa
277
 
                 BASE_PROFILE(cxt->active)->name,
278
 
                 cxt->active->name);
279
 
 
280
 
-       aa_switch_unconfined(cxt);
281
 
+       aa_switch_to_profile(cxt, NULL, 0);
282
 
 }
283
 
 
284
 
 /** taskremove_iter - Iterator to unconfine aa_task_contexts which match cookie
285
 
@@ -100,7 +100,7 @@ static inline void task_replace(struct a
286
 
                 cxt->active->name, cxt->active);
287
 
 
288
 
        if (!cxt->active)
289
 
-               goto out;
290
 
+               return;
291
 
 
292
 
        if (IN_SUBPROFILE(cxt->active)) {
293
 
                struct aa_profile *nactive;
294
 
@@ -112,14 +112,10 @@ static inline void task_replace(struct a
295
 
                if (!nactive)
296
 
                        nactive = aa_dup_profile(new->null_profile);
297
 
 
298
 
-               aa_switch(cxt, nactive);
299
 
+               aa_switch_to_profile(cxt, nactive, cxt->hat_magic);
300
 
                put_aa_profile(nactive);
301
 
-       } else {
302
 
-               aa_switch(cxt, new);
303
 
-       }
304
 
-
305
 
- out:
306
 
-       return;
307
 
+       } else
308
 
+               aa_switch_to_profile(cxt, new, cxt->hat_magic);
309
 
 }
310
 
 
311
 
 /** taskreplace_iter - Iterator to replace a aa_task_context's profile