diff options
author | Lai Jiangshan <laijs@cn.fujitsu.com> | 2008-10-18 23:28:07 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2008-10-20 11:52:38 -0400 |
commit | 47c59803becb55b72b26cdab3838d621a15badc8 (patch) | |
tree | 63711f3e41f46288e2fa18db0b4ed734e9b1f668 /security | |
parent | c012a54ae0b2ee2c73499f54596e0f5257288fec (diff) |
devcgroup: remove spin_lock()
Since we introduced rcu for read side, spin_lock is used only for update.
But we always hold cgroup_lock() when update, so spin_lock() is not need.
Additional cleanup:
1) include linux/rcupdate.h explicitly
2) remove unused variable cur_devcgroup in devcgroup_update_access()
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: "Serge E. Hallyn" <serue@us.ibm.com>
Cc: Paul Menage <menage@google.com>
Cc: James Morris <jmorris@namei.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'security')
-rw-r--r-- | security/device_cgroup.c | 34 |
1 files changed, 5 insertions, 29 deletions
diff --git a/security/device_cgroup.c b/security/device_cgroup.c index d5c15a7c67c3..5ba78701adc3 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c | |||
@@ -1,5 +1,5 @@ | |||
1 | /* | 1 | /* |
2 | * dev_cgroup.c - device cgroup subsystem | 2 | * device_cgroup.c - device cgroup subsystem |
3 | * | 3 | * |
4 | * Copyright 2007 IBM Corp | 4 | * Copyright 2007 IBM Corp |
5 | */ | 5 | */ |
@@ -10,6 +10,7 @@ | |||
10 | #include <linux/list.h> | 10 | #include <linux/list.h> |
11 | #include <linux/uaccess.h> | 11 | #include <linux/uaccess.h> |
12 | #include <linux/seq_file.h> | 12 | #include <linux/seq_file.h> |
13 | #include <linux/rcupdate.h> | ||
13 | 14 | ||
14 | #define ACC_MKNOD 1 | 15 | #define ACC_MKNOD 1 |
15 | #define ACC_READ 2 | 16 | #define ACC_READ 2 |
@@ -22,18 +23,8 @@ | |||
22 | 23 | ||
23 | /* | 24 | /* |
24 | * whitelist locking rules: | 25 | * whitelist locking rules: |
25 | * cgroup_lock() cannot be taken under dev_cgroup->lock. | 26 | * hold cgroup_lock() for update/read. |
26 | * dev_cgroup->lock can be taken with or without cgroup_lock(). | 27 | * hold rcu_read_lock() for read. |
27 | * | ||
28 | * modifications always require cgroup_lock | ||
29 | * modifications to a list which is visible require the | ||
30 | * dev_cgroup->lock *and* cgroup_lock() | ||
31 | * walking the list requires dev_cgroup->lock or cgroup_lock(). | ||
32 | * | ||
33 | * reasoning: dev_whitelist_copy() needs to kmalloc, so needs | ||
34 | * a mutex, which the cgroup_lock() is. Since modifying | ||
35 | * a visible list requires both locks, either lock can be | ||
36 | * taken for walking the list. | ||
37 | */ | 28 | */ |
38 | 29 | ||
39 | struct dev_whitelist_item { | 30 | struct dev_whitelist_item { |
@@ -47,7 +38,6 @@ struct dev_whitelist_item { | |||
47 | struct dev_cgroup { | 38 | struct dev_cgroup { |
48 | struct cgroup_subsys_state css; | 39 | struct cgroup_subsys_state css; |
49 | struct list_head whitelist; | 40 | struct list_head whitelist; |
50 | spinlock_t lock; | ||
51 | }; | 41 | }; |
52 | 42 | ||
53 | static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) | 43 | static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) |
@@ -103,7 +93,6 @@ free_and_exit: | |||
103 | /* Stupid prototype - don't bother combining existing entries */ | 93 | /* Stupid prototype - don't bother combining existing entries */ |
104 | /* | 94 | /* |
105 | * called under cgroup_lock() | 95 | * called under cgroup_lock() |
106 | * since the list is visible to other tasks, we need the spinlock also | ||
107 | */ | 96 | */ |
108 | static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, | 97 | static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, |
109 | struct dev_whitelist_item *wh) | 98 | struct dev_whitelist_item *wh) |
@@ -114,7 +103,6 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, | |||
114 | if (!whcopy) | 103 | if (!whcopy) |
115 | return -ENOMEM; | 104 | return -ENOMEM; |
116 | 105 | ||
117 | spin_lock(&dev_cgroup->lock); | ||
118 | list_for_each_entry(walk, &dev_cgroup->whitelist, list) { | 106 | list_for_each_entry(walk, &dev_cgroup->whitelist, list) { |
119 | if (walk->type != wh->type) | 107 | if (walk->type != wh->type) |
120 | continue; | 108 | continue; |
@@ -130,7 +118,6 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, | |||
130 | 118 | ||
131 | if (whcopy != NULL) | 119 | if (whcopy != NULL) |
132 | list_add_tail_rcu(&whcopy->list, &dev_cgroup->whitelist); | 120 | list_add_tail_rcu(&whcopy->list, &dev_cgroup->whitelist); |
133 | spin_unlock(&dev_cgroup->lock); | ||
134 | return 0; | 121 | return 0; |
135 | } | 122 | } |
136 | 123 | ||
@@ -144,14 +131,12 @@ static void whitelist_item_free(struct rcu_head *rcu) | |||
144 | 131 | ||
145 | /* | 132 | /* |
146 | * called under cgroup_lock() | 133 | * called under cgroup_lock() |
147 | * since the list is visible to other tasks, we need the spinlock also | ||
148 | */ | 134 | */ |
149 | static void dev_whitelist_rm(struct dev_cgroup *dev_cgroup, | 135 | static void dev_whitelist_rm(struct dev_cgroup *dev_cgroup, |
150 | struct dev_whitelist_item *wh) | 136 | struct dev_whitelist_item *wh) |
151 | { | 137 | { |
152 | struct dev_whitelist_item *walk, *tmp; | 138 | struct dev_whitelist_item *walk, *tmp; |
153 | 139 | ||
154 | spin_lock(&dev_cgroup->lock); | ||
155 | list_for_each_entry_safe(walk, tmp, &dev_cgroup->whitelist, list) { | 140 | list_for_each_entry_safe(walk, tmp, &dev_cgroup->whitelist, list) { |
156 | if (walk->type == DEV_ALL) | 141 | if (walk->type == DEV_ALL) |
157 | goto remove; | 142 | goto remove; |
@@ -169,7 +154,6 @@ remove: | |||
169 | call_rcu(&walk->rcu, whitelist_item_free); | 154 | call_rcu(&walk->rcu, whitelist_item_free); |
170 | } | 155 | } |
171 | } | 156 | } |
172 | spin_unlock(&dev_cgroup->lock); | ||
173 | } | 157 | } |
174 | 158 | ||
175 | /* | 159 | /* |
@@ -209,7 +193,6 @@ static struct cgroup_subsys_state *devcgroup_create(struct cgroup_subsys *ss, | |||
209 | } | 193 | } |
210 | } | 194 | } |
211 | 195 | ||
212 | spin_lock_init(&dev_cgroup->lock); | ||
213 | return &dev_cgroup->css; | 196 | return &dev_cgroup->css; |
214 | } | 197 | } |
215 | 198 | ||
@@ -325,15 +308,11 @@ static int parent_has_perm(struct dev_cgroup *childcg, | |||
325 | { | 308 | { |
326 | struct cgroup *pcg = childcg->css.cgroup->parent; | 309 | struct cgroup *pcg = childcg->css.cgroup->parent; |
327 | struct dev_cgroup *parent; | 310 | struct dev_cgroup *parent; |
328 | int ret; | ||
329 | 311 | ||
330 | if (!pcg) | 312 | if (!pcg) |
331 | return 1; | 313 | return 1; |
332 | parent = cgroup_to_devcgroup(pcg); | 314 | parent = cgroup_to_devcgroup(pcg); |
333 | spin_lock(&parent->lock); | 315 | return may_access_whitelist(parent, wh); |
334 | ret = may_access_whitelist(parent, wh); | ||
335 | spin_unlock(&parent->lock); | ||
336 | return ret; | ||
337 | } | 316 | } |
338 | 317 | ||
339 | /* | 318 | /* |
@@ -352,7 +331,6 @@ static int parent_has_perm(struct dev_cgroup *childcg, | |||
352 | static int devcgroup_update_access(struct dev_cgroup *devcgroup, | 331 | static int devcgroup_update_access(struct dev_cgroup *devcgroup, |
353 | int filetype, const char *buffer) | 332 | int filetype, const char *buffer) |
354 | { | 333 | { |
355 | struct dev_cgroup *cur_devcgroup; | ||
356 | const char *b; | 334 | const char *b; |
357 | char *endp; | 335 | char *endp; |
358 | int count; | 336 | int count; |
@@ -361,8 +339,6 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, | |||
361 | if (!capable(CAP_SYS_ADMIN)) | 339 | if (!capable(CAP_SYS_ADMIN)) |
362 | return -EPERM; | 340 | return -EPERM; |
363 | 341 | ||
364 | cur_devcgroup = task_devcgroup(current); | ||
365 | |||
366 | memset(&wh, 0, sizeof(wh)); | 342 | memset(&wh, 0, sizeof(wh)); |
367 | b = buffer; | 343 | b = buffer; |
368 | 344 | ||