Re: [PATCH 1/2] perf: fix find_get_context() vsperf_event_exit_task() race

[Peter Zijlstra]

On Wed, 2011-01-19 at 19:22 +0100, Oleg Nesterov wrote:
> find_get_context() must not install the new perf_event_context if the
> task has already passed perf_event_exit_task().
> 
> If nothing else, this means the memory leak. Initially ctx->refcount == 2,
> it is supposed that perf_event_exit_task_context() should participate and
> do the necessary put_ctx().
> 
> find_lively_task_by_vpid() checks PF_EXITING but this buys nothing, by the
> time we call find_get_context() this task can be already dead. To the point,
> cmpxchg() can succeed when the task has already done the last schedule().
> 
> Change find_get_context() to populate task->perf_event_ctxp[] under
> task->perf_event_mutex, this way we can trust PF_EXITING because
> perf_event_exit_task() takes the same mutex.
> 
> Also, change perf_event_exit_task_context() to use rcu_dereference().
> Probably this is not strictly needed, but with or without this change
> find_get_context() can race with setup_new_exec()->perf_event_exit_task(),
> rcu_dereference() looks better.

I think initially the idea was that this race couldn't happen because by
that time we would be unhashed from the pidhash and thus invisible for
new events, however from what I can make from the exit path we get
unhashed in exit_notify() which is _after_ perf_event_exit_task(), so
yes this looks to be a proper fix.

Acked-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/