linux-kernel - Re: [PATCH v2] perf/core: fix RCU issues with cgroup monitoring mode

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:	Tue, 27 Oct 2015 23:34:20 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Stephane Eranian <eranian@...gle.com>,
	linux-kernel@...r.kernel.org, mingo@...e.hu, ak@...ux.intel.com,
	edumazet@...gle.com, acme@...hat.com
Subject: Re: [PATCH v2] perf/core: fix RCU issues with cgroup monitoring mode

On Tue, Oct 27, 2015 at 01:24:05PM -0700, Eric Dumazet wrote:
> On Tue, 2015-10-27 at 20:25 +0100, Stephane Eranian wrote:
> > This patch eliminates all known RCU violations detected
> > by the RCU  checker (PROVE_RCU). The impact code paths
> > were all related to cgroup mode monitoring and involved
> > access a task's cgrp.
> > 
> > V2 is updated to include suggestions from PeterZ to eliminate
> > some of the warnings without grabbing the rcu_read lock because
> > we know we are already holding the ctx->lock which prevents
> > the cgroup from disappearing while we are accessing it.
> > The trick, as suggested by Peter, is to modify the
> > perf_cgroup_from_task() to take an extra boolean parameter
> > to allow bypassing the lockdep test in the task_subsys_cstate()
> > macros. This patch uses this approach to update all calls the
> > perf_cgroup_from_task().
> > V2 Patch relative to:
> > 8b3c8e6 Revert "rculist: Make list_entry_rcu() use lockless_dereference()"
> > 
> > Signed-off-by: Stephane Eranian <eranian@...gle.com>
> > ---
> 
> 
> Instead trusting caller to provide correct 'safe' boolean,
> what about using lockdep_is_held() ?
> 
> This way, you keep full lockep support.

Because its really really hard to get from a random task to the right
lock in this case :/

The connection is something like:

	task <-> cgroup <-> event <-> ctx <-> lock

So for any given task, we need to find its cgroup (easy, but this
already requires knowing the lock), we need to find all events for that
cgroup and locate the one that is for the cpu the task runs on. Then we
need to find its context and see it the lock is taken.

So aside from if being rather hard, there's also the chicken-egg
problem.

Now I suppose we can pass the right ctx in, and when !NULL use
lock_is_held() on it. But we'd need to validate that the ctx passed
matches the cgroup, and that's not entirely trivial either.

A well :/

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