Re: perf: use-after-free in perf_event_for_each

From: Dmitry Vyukov
Date: Fri Jan 27 2017 - 08:29:54 EST


On Thu, Jan 26, 2017 at 4:39 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Mon, Jan 23, 2017 at 02:30:12PM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> The following program triggers use-after-free in perf_event_for_each:
>> https://gist.githubusercontent.com/dvyukov/f1c354a8356e42f4d0b3d912e1bec956/raw/31d7ecdf6dc2c7327b80ef8581a39c823bbe405d/gistfile1.txt
>>
>> BUG: KASAN: use-after-free in perf_event_for_each_child+0x15f/0x180
>> kernel/events/core.c:4495 at addr ffff8800680ec248
>
> The below seems to fix things for me.

I've run the program again, it crashed machine in <1 min. Then applied
your patch and run the program again, no crashes in 20 minutes. So:

Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>


> ---
> Subject: perf: Fix use-after-free
>
> Dmitry reported a KASAN use-after-free.
>
> XXX: do a changelog
>
> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
> kernel/events/core.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 896aa53..8c1ad98 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -1474,7 +1474,6 @@ ctx_group_list(struct perf_event *event, struct perf_event_context *ctx)
> static void
> list_add_event(struct perf_event *event, struct perf_event_context *ctx)
> {
> -
> lockdep_assert_held(&ctx->lock);
>
> WARN_ON_ONCE(event->attach_state & PERF_ATTACH_CONTEXT);
> @@ -1629,6 +1628,8 @@ static void perf_group_attach(struct perf_event *event)
> {
> struct perf_event *group_leader = event->group_leader, *pos;
>
> + lockdep_assert_held(&event->ctx->lock);
> +
> /*
> * We can have double attach due to group movement in perf_event_open.
> */
> @@ -1702,6 +1703,8 @@ static void perf_group_detach(struct perf_event *event)
> struct perf_event *sibling, *tmp;
> struct list_head *list = NULL;
>
> + lockdep_assert_held(&event->ctx->lock);
> +
> /*
> * We can have double detach due to exit/hot-unplug + close.
> */
> @@ -1900,9 +1903,29 @@ __perf_remove_from_context(struct perf_event *event,
> */
> static void perf_remove_from_context(struct perf_event *event, unsigned long flags)
> {
> - lockdep_assert_held(&event->ctx->mutex);
> + struct perf_event_context *ctx = event->ctx;
> +
> + lockdep_assert_held(&ctx->mutex);
>
> event_function_call(event, __perf_remove_from_context, (void *)flags);
> +
> + /*
> + * The above event_function_call() can NO-OP when it hits
> + * TASK_TOMBSTONE. In that case we must already have been detached
> + * from the context (by perf_event_exit_event()) but the grouping
> + * might still be in-tact.
> + */
> + WARN_ON_ONCE(event->attach_state & PERF_ATTACH_CONTEXT);
> + if ((flags & DETACH_GROUP) &&
> + (event->attach_state & PERF_ATTACH_GROUP)) {
> + /*
> + * Since in that case we cannot possibly be scheduled, simply
> + * detach now.
> + */
> + raw_spin_lock_irq(&ctx->lock);
> + perf_group_detach(event);
> + raw_spin_unlock_irq(&ctx->lock);
> + }
> }
>
> /*