lkml.org 
[lkml]   [2020]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 097/101] perf/core: Fix race in the perf_mmap_close() function
    Date
    From: Jiri Olsa <jolsa@redhat.com>

    commit f91072ed1b7283b13ca57fcfbece5a3b92726143 upstream.

    There's a possible race in perf_mmap_close() when checking ring buffer's
    mmap_count refcount value. The problem is that the mmap_count check is
    not atomic because we call atomic_dec() and atomic_read() separately.

    perf_mmap_close:
    ...
    atomic_dec(&rb->mmap_count);
    ...
    if (atomic_read(&rb->mmap_count))
    goto out_put;

    <ring buffer detach>
    free_uid

    out_put:
    ring_buffer_put(rb); /* could be last */

    The race can happen when we have two (or more) events sharing same ring
    buffer and they go through atomic_dec() and then they both see 0 as refcount
    value later in atomic_read(). Then both will go on and execute code which
    is meant to be run just once.

    The code that detaches ring buffer is probably fine to be executed more
    than once, but the problem is in calling free_uid(), which will later on
    demonstrate in related crashes and refcount warnings, like:

    refcount_t: addition on 0; use-after-free.
    ...
    RIP: 0010:refcount_warn_saturate+0x6d/0xf
    ...
    Call Trace:
    prepare_creds+0x190/0x1e0
    copy_creds+0x35/0x172
    copy_process+0x471/0x1a80
    _do_fork+0x83/0x3a0
    __do_sys_wait4+0x83/0x90
    __do_sys_clone+0x85/0xa0
    do_syscall_64+0x5b/0x1e0
    entry_SYSCALL_64_after_hwframe+0x44/0xa9

    Using atomic decrease and check instead of separated calls.

    Tested-by: Michael Petlan <mpetlan@redhat.com>
    Signed-off-by: Jiri Olsa <jolsa@kernel.org>
    Signed-off-by: Ingo Molnar <mingo@kernel.org>
    Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Acked-by: Namhyung Kim <namhyung@kernel.org>
    Acked-by: Wade Mealing <wmealing@redhat.com>
    Fixes: 9bb5d40cd93c ("perf: Fix mmap() accounting hole");
    Link: https://lore.kernel.org/r/20200916115311.GE2301783@krava
    [sudip: used ring_buffer]
    Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    kernel/events/core.c | 7 ++++---
    1 file changed, 4 insertions(+), 3 deletions(-)

    --- a/kernel/events/core.c
    +++ b/kernel/events/core.c
    @@ -5475,11 +5475,11 @@ static void perf_pmu_output_stop(struct
    static void perf_mmap_close(struct vm_area_struct *vma)
    {
    struct perf_event *event = vma->vm_file->private_data;
    -
    struct ring_buffer *rb = ring_buffer_get(event);
    struct user_struct *mmap_user = rb->mmap_user;
    int mmap_locked = rb->mmap_locked;
    unsigned long size = perf_data_size(rb);
    + bool detach_rest = false;

    if (event->pmu->event_unmapped)
    event->pmu->event_unmapped(event, vma->vm_mm);
    @@ -5510,7 +5510,8 @@ static void perf_mmap_close(struct vm_ar
    mutex_unlock(&event->mmap_mutex);
    }

    - atomic_dec(&rb->mmap_count);
    + if (atomic_dec_and_test(&rb->mmap_count))
    + detach_rest = true;

    if (!atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
    goto out_put;
    @@ -5519,7 +5520,7 @@ static void perf_mmap_close(struct vm_ar
    mutex_unlock(&event->mmap_mutex);

    /* If there's still other mmap()s of this buffer, we're done. */
    - if (atomic_read(&rb->mmap_count))
    + if (!detach_rest)
    goto out_put;

    /*

    \
     
     \ /
      Last update: 2020-11-17 15:10    [W:4.239 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site