• Steven Rostedt (Google)'s avatar
    tracing/ring-buffer: Fix wait_on_pipe() race · 2aa043a5
    Steven Rostedt (Google) authored
    When the trace_pipe_raw file is closed, there should be no new readers on
    the file descriptor. This is mostly handled with the waking and wait_index
    fields of the iterator. But there's still a slight race.
    
         CPU 0                              CPU 1
         -----                              -----
                                       wait_index++;
       index = wait_index;
                                       ring_buffer_wake_waiters();
       wait_on_pipe()
         ring_buffer_wait();
    
    The ring_buffer_wait() will miss the wakeup from CPU 1. The problem is
    that the ring_buffer_wait() needs the logic of:
    
            prepare_to_wait();
            if (!condition)
                    schedule();
    
    Where the missing condition check is the iter->wait_index update.
    
    Have the ring_buffer_wait() take a conditional callback function and a
    data parameter that can be used within the wait_event_interruptible() of
    the ring_buffer_wait() function.
    
    In wait_on_pipe(), pass a condition function that will check if the
    wait_index has been updated, if it has, it will return true to break out
    of the wait_event_interruptible() loop.
    
    Create a new field "closed" in the trace_iterator and set it in the
    .flush() callback before calling ring_buffer_wake_waiters().
    This will keep any new readers from waiting on a closed file descriptor.
    
    Have the wait_on_pipe() condition callback also check the closed field.
    
    Change the wait_index field of the trace_iterator to atomic_t. There's no
    reason it needs to be 'long' and making it atomic and using
    atomic_read_acquire() and atomic_fetch_inc_release() will provide the
    necessary memory barriers.
    
    Add a "woken" flag to tracing_buffers_splice_read() to exit the loop after
    one more try to fetch data. That is, if it waited for data and something
    woke it up, it should try to collect any new data and then exit back to
    user space.
    
    Link: https://lore.kernel.org/linux-trace-kernel/CAHk-=wgsNgewHFxZAJiAQznwPMqEtQmi1waeS2O1v6L4c_Um5A@mail.gmail.com/
    Link: https://lore.kernel.org/linux-trace-kernel/20240312121703.557950713@goodmis.org
    
    Cc: stable@vger.kernel.org
    Cc: Masami Hiramatsu <mhiramat@kernel.org>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: linke li <lilinke99@qq.com>
    Cc: Rabin Vincent <rabin@rab.in>
    Fixes: f3ddb74a ("tracing: Wake up ring buffer waiters on closing of the file")
    Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
    2aa043a5
trace.c 264 KB