perf: Fix sys_perf_event_open() race against self
commit3ac6487e58
upstream. Norbert reported that it's possible to race sys_perf_event_open() such that the looser ends up in another context from the group leader, triggering many WARNs. The move_group case checks for races against itself, but the !move_group case doesn't, seemingly relying on the previous group_leader->ctx == ctx check. However, that check is racy due to not holding any locks at that time. Therefore, re-check the result after acquiring locks and bailing if they no longer match. Additionally, clarify the not_move_group case from the move_group-vs-move_group race. Fixes:f63a8daa58
("perf: Fix event->ctx locking") Reported-by: Norbert Slusarek <nslusarek@gmx.net> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Tao Wu <tallwu@tencent.com> Reviewed-by: Alex Shi <alexsshi@tencent.com>
This commit is contained in:
parent
5059594a10
commit
1e450e34ef
|
@ -11127,6 +11127,9 @@ SYSCALL_DEFINE5(perf_event_open,
|
||||||
* Do not allow to attach to a group in a different task
|
* Do not allow to attach to a group in a different task
|
||||||
* or CPU context. If we're moving SW events, we'll fix
|
* or CPU context. If we're moving SW events, we'll fix
|
||||||
* this up later, so allow that.
|
* this up later, so allow that.
|
||||||
|
*
|
||||||
|
* Racy, not holding group_leader->ctx->mutex, see comment with
|
||||||
|
* perf_event_ctx_lock().
|
||||||
*/
|
*/
|
||||||
if (!move_group && group_leader->ctx != ctx)
|
if (!move_group && group_leader->ctx != ctx)
|
||||||
goto err_context;
|
goto err_context;
|
||||||
|
@ -11194,6 +11197,7 @@ SYSCALL_DEFINE5(perf_event_open,
|
||||||
} else {
|
} else {
|
||||||
perf_event_ctx_unlock(group_leader, gctx);
|
perf_event_ctx_unlock(group_leader, gctx);
|
||||||
move_group = 0;
|
move_group = 0;
|
||||||
|
goto not_move_group;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -11210,7 +11214,17 @@ SYSCALL_DEFINE5(perf_event_open,
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
mutex_lock(&ctx->mutex);
|
mutex_lock(&ctx->mutex);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Now that we hold ctx->lock, (re)validate group_leader->ctx == ctx,
|
||||||
|
* see the group_leader && !move_group test earlier.
|
||||||
|
*/
|
||||||
|
if (group_leader && group_leader->ctx != ctx) {
|
||||||
|
err = -EINVAL;
|
||||||
|
goto err_locked;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
not_move_group:
|
||||||
|
|
||||||
if (ctx->task == TASK_TOMBSTONE) {
|
if (ctx->task == TASK_TOMBSTONE) {
|
||||||
err = -ESRCH;
|
err = -ESRCH;
|
||||||
|
|
Loading…
Reference in New Issue