Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

tracing: Fix buffer_ref pipe ops

This fixes multiple issues in buffer_pipe_buf_ops:

- The ->steal() handler must not return zero unless the pipe buffer has
the only reference to the page. But generic_pipe_buf_steal() assumes
that every reference to the pipe is tracked by the page's refcount,
which isn't true for these buffers - buffer_pipe_buf_get(), which
duplicates a buffer, doesn't touch the page's refcount.
Fix it by using generic_pipe_buf_nosteal(), which refuses every
attempted theft. It should be easy to actually support ->steal, but the
only current users of pipe_buf_steal() are the virtio console and FUSE,
and they also only use it as an optimization. So it's probably not worth
the effort.
- The ->get() and ->release() handlers can be invoked concurrently on pipe
buffers backed by the same struct buffer_ref. Make them safe against
concurrency by using refcount_t.
- The pointers stored in ->private were only zeroed out when the last
reference to the buffer_ref was dropped. As far as I know, this
shouldn't be necessary anyway, but if we do it, let's always do it.

Link: http://lkml.kernel.org/r/20190404215925.253531-1-jannh@google.com

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: stable@vger.kernel.org
Fixes: 73a757e63114d ("ring-buffer: Return reader page back into existing ring buffer")
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

authored by

Jann Horn and committed by
Steven Rostedt (VMware)
b9872226 79a3aaa7

+17 -16
+2 -2
fs/splice.c
··· 330 330 .get = generic_pipe_buf_get, 331 331 }; 332 332 333 - static int generic_pipe_buf_nosteal(struct pipe_inode_info *pipe, 334 - struct pipe_buffer *buf) 333 + int generic_pipe_buf_nosteal(struct pipe_inode_info *pipe, 334 + struct pipe_buffer *buf) 335 335 { 336 336 return 1; 337 337 }
+1
include/linux/pipe_fs_i.h
··· 174 174 void generic_pipe_buf_get(struct pipe_inode_info *, struct pipe_buffer *); 175 175 int generic_pipe_buf_confirm(struct pipe_inode_info *, struct pipe_buffer *); 176 176 int generic_pipe_buf_steal(struct pipe_inode_info *, struct pipe_buffer *); 177 + int generic_pipe_buf_nosteal(struct pipe_inode_info *, struct pipe_buffer *); 177 178 void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *); 178 179 void pipe_buf_mark_unmergeable(struct pipe_buffer *buf); 179 180
+14 -14
kernel/trace/trace.c
··· 7025 7025 struct ring_buffer *buffer; 7026 7026 void *page; 7027 7027 int cpu; 7028 - int ref; 7028 + refcount_t refcount; 7029 7029 }; 7030 + 7031 + static void buffer_ref_release(struct buffer_ref *ref) 7032 + { 7033 + if (!refcount_dec_and_test(&ref->refcount)) 7034 + return; 7035 + ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page); 7036 + kfree(ref); 7037 + } 7030 7038 7031 7039 static void buffer_pipe_buf_release(struct pipe_inode_info *pipe, 7032 7040 struct pipe_buffer *buf) 7033 7041 { 7034 7042 struct buffer_ref *ref = (struct buffer_ref *)buf->private; 7035 7043 7036 - if (--ref->ref) 7037 - return; 7038 - 7039 - ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page); 7040 - kfree(ref); 7044 + buffer_ref_release(ref); 7041 7045 buf->private = 0; 7042 7046 } 7043 7047 ··· 7050 7046 { 7051 7047 struct buffer_ref *ref = (struct buffer_ref *)buf->private; 7052 7048 7053 - ref->ref++; 7049 + refcount_inc(&ref->refcount); 7054 7050 } 7055 7051 7056 7052 /* Pipe buffer operations for a buffer. */ 7057 7053 static const struct pipe_buf_operations buffer_pipe_buf_ops = { 7058 7054 .confirm = generic_pipe_buf_confirm, 7059 7055 .release = buffer_pipe_buf_release, 7060 - .steal = generic_pipe_buf_steal, 7056 + .steal = generic_pipe_buf_nosteal, 7061 7057 .get = buffer_pipe_buf_get, 7062 7058 }; 7063 7059 ··· 7070 7066 struct buffer_ref *ref = 7071 7067 (struct buffer_ref *)spd->partial[i].private; 7072 7068 7073 - if (--ref->ref) 7074 - return; 7075 - 7076 - ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page); 7077 - kfree(ref); 7069 + buffer_ref_release(ref); 7078 7070 spd->partial[i].private = 0; 7079 7071 } 7080 7072 ··· 7125 7125 break; 7126 7126 } 7127 7127 7128 - ref->ref = 1; 7128 + refcount_set(&ref->refcount, 1); 7129 7129 ref->buffer = iter->trace_buffer->buffer; 7130 7130 ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file); 7131 7131 if (IS_ERR(ref->page)) {