Messages in this thread | | | Date | Tue, 14 Mar 2017 16:26:25 +0100 | From | Peter Zijlstra <> | Subject | Re: perf: use-after-free in perf_release |
| |
On Tue, Mar 14, 2017 at 04:19:10PM +0100, Oleg Nesterov wrote: > On 03/14, Peter Zijlstra wrote: > > > > mutex_unlock(ctx->lock); > > put_ctx() /* >0 */ > > free_task(); > > mutex_lock(ctx->lock); > > mutex_lock(A->child_mutex); > > /* ... */ > > mutex_unlock(A->child_mutex); > > mutex_unlock(ctx->lock) > > put_ctx() /* 0 */ > > ctx->task && !TOMBSTONE > > put_task_struct() /* UAF */ > > > > > > Something like that, right? > > Yes, exactly. > > > Let me see if it makes sense to retain perf_event_free_task() at all; > > maybe we should always do perf_event_exit_task(). > > Yes, perhaps... but this needs changes too. Say, WARN_ON_ONCE(child != current) > in perf_event_exit_task_context(). And even perf_event_task(new => F) does not > look right in this case. In fact it would be simply buggy to do this, this task > was not fully constructed yet, so even perf_event_pid(task) is not safe.
Yeah; there's a fair amount of stuff like that. I'm afraid crafting exceptions for all that will just end up with more of a mess than we safe by merging the two :/
A well.. I'll go do the 'trivial' patch then.
| |