From: Pavel Emelyanov Date: Fri, 25 Jul 2008 08:47:07 +0000 (-0700) Subject: devcgroup: relax white-list protection down to RCU X-Git-Tag: v2.6.27-rc1~380 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4efd1a1b2f09a4b746dd9dc057986c6dadcb1317;p=linux-2.6 devcgroup: relax white-list protection down to RCU Currently this list is protected with a simple spinlock, even for reading from one. This is OK, but can be better. Actually I want it to be better very much, since after replacing the OpenVZ device permissions engine with the cgroup-based one I noticed, that we set 12 default device permissions for each newly created container (for /dev/null, full, terminals, ect devices), and people sometimes have up to 20 perms more, so traversing the ~30-40 elements list under a spinlock doesn't seem very good. Here's the RCU protection for white-list - dev_whitelist_item-s are added and removed under the devcg->lock, but are looked up in permissions checking under the rcu_read_lock. Signed-off-by: Pavel Emelyanov Acked-by: Serge Hallyn Cc: Balbir Singh Cc: Paul Menage Cc: "Paul E. McKenney" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 236fffa9d0..9da3532726 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -41,6 +41,7 @@ struct dev_whitelist_item { short type; short access; struct list_head list; + struct rcu_head rcu; }; struct dev_cgroup { @@ -133,11 +134,19 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, } if (whcopy != NULL) - list_add_tail(&whcopy->list, &dev_cgroup->whitelist); + list_add_tail_rcu(&whcopy->list, &dev_cgroup->whitelist); spin_unlock(&dev_cgroup->lock); return 0; } +static void whitelist_item_free(struct rcu_head *rcu) +{ + struct dev_whitelist_item *item; + + item = container_of(rcu, struct dev_whitelist_item, rcu); + kfree(item); +} + /* * called under cgroup_lock() * since the list is visible to other tasks, we need the spinlock also @@ -161,8 +170,8 @@ static void dev_whitelist_rm(struct dev_cgroup *dev_cgroup, remove: walk->access &= ~wh->access; if (!walk->access) { - list_del(&walk->list); - kfree(walk); + list_del_rcu(&walk->list); + call_rcu(&walk->rcu, whitelist_item_free); } } spin_unlock(&dev_cgroup->lock); @@ -269,15 +278,15 @@ static int devcgroup_seq_read(struct cgroup *cgroup, struct cftype *cft, struct dev_whitelist_item *wh; char maj[MAJMINLEN], min[MAJMINLEN], acc[ACCLEN]; - spin_lock(&devcgroup->lock); - list_for_each_entry(wh, &devcgroup->whitelist, list) { + rcu_read_lock(); + list_for_each_entry_rcu(wh, &devcgroup->whitelist, list) { set_access(acc, wh->access); set_majmin(maj, wh->major); set_majmin(min, wh->minor); seq_printf(m, "%c %s:%s %s\n", type_to_char(wh->type), maj, min, acc); } - spin_unlock(&devcgroup->lock); + rcu_read_unlock(); return 0; } @@ -510,8 +519,8 @@ int devcgroup_inode_permission(struct inode *inode, int mask) if (!dev_cgroup) return 0; - spin_lock(&dev_cgroup->lock); - list_for_each_entry(wh, &dev_cgroup->whitelist, list) { + rcu_read_lock(); + list_for_each_entry_rcu(wh, &dev_cgroup->whitelist, list) { if (wh->type & DEV_ALL) goto acc_check; if ((wh->type & DEV_BLOCK) && !S_ISBLK(inode->i_mode)) @@ -527,10 +536,10 @@ acc_check: continue; if ((mask & MAY_READ) && !(wh->access & ACC_READ)) continue; - spin_unlock(&dev_cgroup->lock); + rcu_read_unlock(); return 0; } - spin_unlock(&dev_cgroup->lock); + rcu_read_unlock(); return -EPERM; } @@ -545,7 +554,7 @@ int devcgroup_inode_mknod(int mode, dev_t dev) if (!dev_cgroup) return 0; - spin_lock(&dev_cgroup->lock); + rcu_read_lock(); list_for_each_entry(wh, &dev_cgroup->whitelist, list) { if (wh->type & DEV_ALL) goto acc_check; @@ -560,9 +569,9 @@ int devcgroup_inode_mknod(int mode, dev_t dev) acc_check: if (!(wh->access & ACC_MKNOD)) continue; - spin_unlock(&dev_cgroup->lock); + rcu_read_unlock(); return 0; } - spin_unlock(&dev_cgroup->lock); + rcu_read_unlock(); return -EPERM; }