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

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:	Thu, 30 Oct 2008 00:41:39 -0700
From:	"Paul Menage" <menage@...gle.com>
To:	"Li Zefan" <lizf@...fujitsu.com>
Cc:	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Peter Zijlstra" <a.p.zijlstra@...llo.nl>,
	"Ingo Molnar" <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	"Linux Containers" <containers@...ts.linux-foundation.org>
Subject: Re: [PATCH] cgroup: fix invalid cgrp->dentry before cgroup has been completely removed

On Thu, Oct 30, 2008 at 12:23 AM, Li Zefan <lizf@...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.

Thanks.

Acked-by: Paul Menage <menage@...gle.com>



>
> The bug can be reproduced:
>
>  # cat test.sh
>  #!/bin/sh
>  mount -t cgroup -o cpu xxx /mnt
>  for (( ; ; ))
>  {
>        mkdir /mnt/sub
>        rmdir /mnt/sub
>  }
>  # ./test.sh &
>  # cat /proc/sched_debug
>
> BUG: unable to handle kernel NULL pointer dereference at 00000038
> IP: [<c045a47f>] cgroup_path+0x39/0x90
> ...
> Call Trace:
>  [<c0420344>] ? print_cfs_rq+0x6e/0x75d
>  [<c0421160>] ? sched_debug_show+0x72d/0xc1e
> ...
>
> Signed-off-by: Li Zefan <lizf@...fujitsu.com>
> ---
>  kernel/cgroup.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 35eebd5..358e775 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -2497,7 +2497,6 @@ static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry)
>        list_del(&cgrp->sibling);
>        spin_lock(&cgrp->dentry->d_lock);
>        d = dget(cgrp->dentry);
> -       cgrp->dentry = NULL;
>        spin_unlock(&d->d_lock);
>
>        cgroup_d_remove_dir(d);
> --
> 1.5.4.rc3
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/