lkml.org 
[lkml]   [2019]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC PATCH] perf: Paper over the hw.target problems
    Date
    First, we have a race between perf_event_release_kernel() and
    perf_free_event(), which happens when parent's event is released while the
    child's fork fails (because of a fatal signal, for example), that looks
    like this:

    cpu X cpu Y
    ----- -----
    copy_process() error path
    perf_release(parent) +->perf_event_free_task()
    +-> lock(child_ctx->mutex) | |
    +-> remove_from_context(child) | |
    +-> unlock(child_ctx->mutex) | |
    | | +-> lock(child_ctx->mutex)
    | | +-> unlock(child_ctx->mutex)
    | +-> free_task(child_task)
    +-> put_task_struct(child_task)

    Technically, we're still holding a reference to the task via
    parent->hw.target, that's not stopping free_task(), so we end up poking at
    free'd memory, as is pointed out by KASAN in the syzkaller report (see Link
    below). The straightforward fix is to drop the hw.target reference while
    the task is still around.

    Therein lies the second problem: the users of hw.target (uprobe) assume
    that it's around at ->destroy() callback time, where they use it for
    context. So, in order to not break the uprobe teardown and avoid leaking
    stuff, we need to call ->destroy() at the same time.

    This patch fixes the race and the subsequent fallout by doing both these
    things at remove_from_context time.

    Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Link: https://syzkaller.appspot.com/bug?extid=a24c397a29ad22d86c98
    Reported-by: syzbot+a24c397a29ad22d86c98@syzkaller.appspotmail.com
    ---
    kernel/events/core.c | 21 +++++++++++++++++++++
    1 file changed, 21 insertions(+)

    diff --git a/kernel/events/core.c b/kernel/events/core.c
    index 36b8320590e8..640695d114f8 100644
    --- a/kernel/events/core.c
    +++ b/kernel/events/core.c
    @@ -2105,6 +2105,27 @@ static void perf_remove_from_context(struct perf_event *event, unsigned long fla

    event_function_call(event, __perf_remove_from_context, (void *)flags);

    + /*
    + * This is as passable as any hw.target handling out there;
    + * hw.target implies task context, therefore, no migration.
    + * Which means that we can only get here at the teardown.
    + */
    + if (event->hw.target) {
    + /*
    + * Now, the problem with, say uprobes, is that they
    + * use hw.target for context in their ->destroy()
    + * callbacks. Supposedly, they may need to poke at
    + * its contents, so better call it while we still
    + * have the task.
    + */
    + if (event->destroy) {
    + event->destroy(event);
    + event->destroy = NULL;
    + }
    + put_task_struct(event->hw.target);
    + event->hw.target = NULL;
    + }
    +
    /*
    * The above event_function_call() can NO-OP when it hits
    * TASK_TOMBSTONE. In that case we must already have been detached
    --
    2.20.1
    \
     
     \ /
      Last update: 2019-02-28 15:02    [W:5.917 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site