Re: [PATCH] cgroup: fix invalid cgrp->dentry before cgroup has been completely removed

[Li Zefan <lizf@cn.fujitsu.com>]

Paul Menage wrote:
> On Thu, Oct 30, 2008 at 12:23 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>> This fixes oops when reading /proc/sched_debug.
>>
>> A cgroup won't be removed completely until finishing cgroup_diput(), so we
>> shouldn't invalidate cgrp->dentry in cgroup_rmdir(). Otherwise, when a
>> group is being removed while cgroup_path() gets called, we may trigger NULL
>> dereference BUG.
> 
> Clearly a bug if it can hit a NULL dereference. But clearing the
> dentry to NULL is something that cgroups inherited from cpusets - it
> looks OK to remove it, but I'm mildly nervous.
> 
> Directly after the code in your patch, we dput() the dentry. So
> theoretically it could be released any time after that. But I guess
> that as soon as it *is* released, cgroup_diput() will be called as
> part of that cleanup, at which point any subsystems should drop any
> pointers they have to the cgroup or the dentry. So I guess it should
> be OK.
> 

The bug is:

cgroup           cpu_subsystem
------------------------------------

cgroup_remove()
                 print_cfs_stats()
                 print_cfs_rq()
cgroup_diput()
                 cpu_cgroup_destroy()

I think a different fix is to add pre_destroy() method to cpu_subsystem,
and move some code from sched_destroy_group() to that method.

But I didn't try it out.