lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 6.0 156/862] ring-buffer: Fix race between reset page and reading page
    Date
    From: Steven Rostedt (Google) <rostedt@goodmis.org>

    commit a0fcaaed0c46cf9399d3a2d6e0c87ddb3df0e044 upstream.

    The ring buffer is broken up into sub buffers (currently of page size).
    Each sub buffer has a pointer to its "tail" (the last event written to the
    sub buffer). When a new event is requested, the tail is locally
    incremented to cover the size of the new event. This is done in a way that
    there is no need for locking.

    If the tail goes past the end of the sub buffer, the process of moving to
    the next sub buffer takes place. After setting the current sub buffer to
    the next one, the previous one that had the tail go passed the end of the
    sub buffer needs to be reset back to the original tail location (before
    the new event was requested) and the rest of the sub buffer needs to be
    "padded".

    The race happens when a reader takes control of the sub buffer. As readers
    do a "swap" of sub buffers from the ring buffer to get exclusive access to
    the sub buffer, it replaces the "head" sub buffer with an empty sub buffer
    that goes back into the writable portion of the ring buffer. This swap can
    happen as soon as the writer moves to the next sub buffer and before it
    updates the last sub buffer with padding.

    Because the sub buffer can be released to the reader while the writer is
    still updating the padding, it is possible for the reader to see the event
    that goes past the end of the sub buffer. This can cause obvious issues.

    To fix this, add a few memory barriers so that the reader definitely sees
    the updates to the sub buffer, and also waits until the writer has put
    back the "tail" of the sub buffer back to the last event that was written
    on it.

    To be paranoid, it will only spin for 1 second, otherwise it will
    warn and shutdown the ring buffer code. 1 second should be enough as
    the writer does have preemption disabled. If the writer doesn't move
    within 1 second (with preemption disabled) something is horribly
    wrong. No interrupt should last 1 second!

    Link: https://lore.kernel.org/all/20220830120854.7545-1-jiazi.li@transsion.com/
    Link: https://bugzilla.kernel.org/show_bug.cgi?id=216369
    Link: https://lkml.kernel.org/r/20220929104909.0650a36c@gandalf.local.home

    Cc: Ingo Molnar <mingo@kernel.org>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: stable@vger.kernel.org
    Fixes: c7b0930857e22 ("ring-buffer: prevent adding write in discarded area")
    Reported-by: Jiazi.Li <jiazi.li@transsion.com>
    Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    kernel/trace/ring_buffer.c | 33 +++++++++++++++++++++++++++++++++
    1 file changed, 33 insertions(+)

    --- a/kernel/trace/ring_buffer.c
    +++ b/kernel/trace/ring_buffer.c
    @@ -2648,6 +2648,9 @@ rb_reset_tail(struct ring_buffer_per_cpu
    /* Mark the rest of the page with padding */
    rb_event_set_padding(event);

    + /* Make sure the padding is visible before the write update */
    + smp_wmb();
    +
    /* Set the write back to the previous setting */
    local_sub(length, &tail_page->write);
    return;
    @@ -2659,6 +2662,9 @@ rb_reset_tail(struct ring_buffer_per_cpu
    /* time delta must be non zero */
    event->time_delta = 1;

    + /* Make sure the padding is visible before the tail_page->write update */
    + smp_wmb();
    +
    /* Set write to end of buffer */
    length = (tail + length) - BUF_PAGE_SIZE;
    local_sub(length, &tail_page->write);
    @@ -4627,6 +4633,33 @@ rb_get_reader_page(struct ring_buffer_pe
    arch_spin_unlock(&cpu_buffer->lock);
    local_irq_restore(flags);

    + /*
    + * The writer has preempt disable, wait for it. But not forever
    + * Although, 1 second is pretty much "forever"
    + */
    +#define USECS_WAIT 1000000
    + for (nr_loops = 0; nr_loops < USECS_WAIT; nr_loops++) {
    + /* If the write is past the end of page, a writer is still updating it */
    + if (likely(!reader || rb_page_write(reader) <= BUF_PAGE_SIZE))
    + break;
    +
    + udelay(1);
    +
    + /* Get the latest version of the reader write value */
    + smp_rmb();
    + }
    +
    + /* The writer is not moving forward? Something is wrong */
    + if (RB_WARN_ON(cpu_buffer, nr_loops == USECS_WAIT))
    + reader = NULL;
    +
    + /*
    + * Make sure we see any padding after the write update
    + * (see rb_reset_tail())
    + */
    + smp_rmb();
    +
    +
    return reader;
    }


    \
     
     \ /
      Last update: 2022-10-19 10:52    [W:2.489 / U:0.696 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site