netdev - Re: [Devel] Re: [RFC][PATCH] IP address restricting cgroup subsystem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 12 Jan 2009 10:20:16 +0800
From:	Li Zefan <lizf@...fujitsu.com>
To:	Paul Menage <menage@...gle.com>
CC:	Grzegorz Nosek <root@...aldomain.pl>, containers@...ts.osdl.org,
	netdev@...r.kernel.org
Subject: Re: [Devel] Re: [RFC][PATCH] IP address restricting cgroup subsystem

Paul Menage wrote:
> On Fri, Jan 9, 2009 at 8:50 PM, Li Zefan <lizf@...fujitsu.com> wrote:
>>> For checking the "children" list, you can just lock
>>> ipaddr_subsys.hierarchy_mutex.
>>>
>> Unfortunately hierarchy_mutex can't be used here, since hierarchy_mutex
>> doesn't protect subsys's create() method, and the create() will access
>> parent cgroup's data.
>>
> 
> But that can be solved by putting a spinlock in the ipaddr_cgroup
> structure and taking it in the write handler (and the connect/bind
> handlers, which should also be using RCU), and taking the parent
> structure's lock before copying from it in the create callback. No
> need for something as heavy as cgroup_lock().
> 

Still won't work. :(

================
lock(ipaddr_subsys.hierarchy_mutex);

if (list_empty(&cgrp->children) &&
    parent->ipv4_addr == INADDR_ANY)
						create()
						  parent->spin_lock();
						  cgrp->ipv4_addr = parent->ipv4_addr;
						  parent->spin_unlock();
	ipcgroup->spin_lock();
	ipcgroup->ipv4_addr = new_addr;
	ipcgroup->spin_unlock();

unlock(ipaddr_subsys.hierarchy_mutex);
						list_add(&cgrp->sibling, &cgrp->parent->children);
================

And neither can it work to hold hierarchy_mutex in subsys's create().

I think the only way to make hierarchy_mutex works for this issue is:

@@ -2403,16 +2403,18 @@ static long cgroup_create(struct cgroup *parent, struct
        if (notify_on_release(parent))
                set_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags);

+       cgroup_lock_hierarchy(root);
+
        for_each_subsys(root, ss) {
                struct cgroup_subsys_state *css = ss->create(ss, cgrp);
                if (IS_ERR(css)) {
+                       cgroup_unlock_hierarchy(root);
                        err = PTR_ERR(css);
                        goto err_destroy;
                }
                init_cgroup_css(css, ss, cgrp);
        }

-       cgroup_lock_hierarchy(root);
        list_add(&cgrp->sibling, &cgrp->parent->children);
        cgroup_unlock_hierarchy(root);
        root->number_of_cgroups++;


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html