linux-kernel - Re: query: [PATCH 2/2] cgroup: Remove call to synchronize_rcu in cgroup_attach

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]

Message-ID: <1304080487.21536.16.camel@marge.simson.net>
Date:	Fri, 29 Apr 2011 14:34:47 +0200
From:	Mike Galbraith <efault@....de>
To:	Paul Menage <menage@...gle.com>
Cc:	Li Zefan <lizf@...fujitsu.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Colin Cross <ccross@...roid.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: query: [PATCH 2/2] cgroup: Remove call to synchronize_rcu in
 cgroup_attach_task

(ok crickets, keep the noise down please)

On Thu, 2011-04-28 at 11:38 +0200, Mike Galbraith wrote:

> The explosions are because the logic to snag rmdir() should anyone grab
> a reference will let us zip right through and free a cgroup while
> there's a destruction in flight.  Adding a cgrp->count check before
> trying to cgroup_clear_css_refs() prevents the explosions, but that
> leaves RCU grace periods still embedded in userspace.
> 
> So...
> 
> I bent up put_css_set() a bit to try to destroy synchronously on final
> put if possible, so rmdir() will only be snagged if that fails.  The
> thing seems to be working, so I'll show it.  Readers (beware) may notice
> some gratuitous looking chicken scratches.  Just ignore those, and move
> along smartly to the suggesting a much better way part, for surely one
> must exist. 

Hi Self, (*howdy*)

You might try the below.  No weird gyrations to accomplish the same
thing, and I see no slub debug gripes, no list debug gripes, nada.

Makes one wonder what these things do for a living.

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 25c7eb5..b8c64bf 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -826,13 +826,6 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode)
 		struct cgroup *cgrp = dentry->d_fsdata;
 		struct cgroup_subsys *ss;
 		BUG_ON(!(cgroup_is_removed(cgrp)));
-		/* It's possible for external users to be holding css
-		 * reference counts on a cgroup; css_put() needs to
-		 * be able to access the cgroup after decrementing
-		 * the reference count in order to know if it needs to
-		 * queue the cgroup to be handled by the release
-		 * agent */
-		synchronize_rcu();
 
 		mutex_lock(&cgroup_mutex);
 		/*
@@ -1822,7 +1815,6 @@ int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
 			ss->attach(ss, cgrp, oldcgrp, tsk, false);
 	}
 	set_bit(CGRP_RELEASABLE, &oldcgrp->flags);
-	synchronize_rcu();
 	put_css_set(cg);
 
 	/*


--
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/