Hello, David. On Tue, Jun 13, 2017 at 03:27:27PM +0100, David Howells wrote: > I'm having a go at converting cgroups to use my fs_context stuff, and I've > spotted a potential bug in the existing code. In cgroup_do_mount(), there is > the following snippet: > > mutex_lock(&cgroup_mutex); > spin_lock_irq(&css_set_lock); > > cgrp = cset_cgroup_from_root(ns->root_cset, root); > > spin_unlock_irq(&css_set_lock); > mutex_unlock(&cgroup_mutex); > > nsdentry = kernfs_node_dentry(cgrp->kn, dentry->d_sb); > > Given that locks must be taken to call cset_cgroup_from_root(), is the value > of cgrp trustworthy once the locks are dropped? Heh, yeah, that does look suspicious. The css_set_lock is protecting the list structure itself and cgroup_mutex is required to exclude cgroup destruction and migration; however, the ns is pinning the root_cset which is immutable and in turn pins the cgroups which are associated with it, so the cgroup will stay pinned even after the mutex is dropped. The mutex lock there is to satisfy the lockdep assertion inside cset_cgroup_from_root(). We definitely should add a comment there explaining what's going on. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
