]>
Commit | Line | Data |
---|---|---|
a47dbcf1 GKH |
1 | From b987222654f84f7b4ca95b3a55eca784cb30235b Mon Sep 17 00:00:00 2001 |
2 | From: Jann Horn <jannh@google.com> | |
3 | Date: Thu, 4 Apr 2019 23:59:25 +0200 | |
4 | Subject: tracing: Fix buffer_ref pipe ops | |
5 | ||
6 | From: Jann Horn <jannh@google.com> | |
7 | ||
8 | commit b987222654f84f7b4ca95b3a55eca784cb30235b upstream. | |
9 | ||
10 | This fixes multiple issues in buffer_pipe_buf_ops: | |
11 | ||
12 | - The ->steal() handler must not return zero unless the pipe buffer has | |
13 | the only reference to the page. But generic_pipe_buf_steal() assumes | |
14 | that every reference to the pipe is tracked by the page's refcount, | |
15 | which isn't true for these buffers - buffer_pipe_buf_get(), which | |
16 | duplicates a buffer, doesn't touch the page's refcount. | |
17 | Fix it by using generic_pipe_buf_nosteal(), which refuses every | |
18 | attempted theft. It should be easy to actually support ->steal, but the | |
19 | only current users of pipe_buf_steal() are the virtio console and FUSE, | |
20 | and they also only use it as an optimization. So it's probably not worth | |
21 | the effort. | |
22 | - The ->get() and ->release() handlers can be invoked concurrently on pipe | |
23 | buffers backed by the same struct buffer_ref. Make them safe against | |
24 | concurrency by using refcount_t. | |
25 | - The pointers stored in ->private were only zeroed out when the last | |
26 | reference to the buffer_ref was dropped. As far as I know, this | |
27 | shouldn't be necessary anyway, but if we do it, let's always do it. | |
28 | ||
29 | Link: http://lkml.kernel.org/r/20190404215925.253531-1-jannh@google.com | |
30 | ||
31 | Cc: Ingo Molnar <mingo@redhat.com> | |
32 | Cc: Masami Hiramatsu <mhiramat@kernel.org> | |
33 | Cc: Al Viro <viro@zeniv.linux.org.uk> | |
34 | Cc: stable@vger.kernel.org | |
35 | Fixes: 73a757e63114d ("ring-buffer: Return reader page back into existing ring buffer") | |
36 | Signed-off-by: Jann Horn <jannh@google.com> | |
37 | Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> | |
38 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
39 | ||
40 | --- | |
41 | fs/splice.c | 4 ++-- | |
42 | include/linux/pipe_fs_i.h | 1 + | |
43 | kernel/trace/trace.c | 28 ++++++++++++++-------------- | |
44 | 3 files changed, 17 insertions(+), 16 deletions(-) | |
45 | ||
46 | --- a/fs/splice.c | |
47 | +++ b/fs/splice.c | |
48 | @@ -333,8 +333,8 @@ const struct pipe_buf_operations default | |
49 | .get = generic_pipe_buf_get, | |
50 | }; | |
51 | ||
52 | -static int generic_pipe_buf_nosteal(struct pipe_inode_info *pipe, | |
53 | - struct pipe_buffer *buf) | |
54 | +int generic_pipe_buf_nosteal(struct pipe_inode_info *pipe, | |
55 | + struct pipe_buffer *buf) | |
56 | { | |
57 | return 1; | |
58 | } | |
59 | --- a/include/linux/pipe_fs_i.h | |
60 | +++ b/include/linux/pipe_fs_i.h | |
61 | @@ -181,6 +181,7 @@ void free_pipe_info(struct pipe_inode_in | |
62 | void generic_pipe_buf_get(struct pipe_inode_info *, struct pipe_buffer *); | |
63 | int generic_pipe_buf_confirm(struct pipe_inode_info *, struct pipe_buffer *); | |
64 | int generic_pipe_buf_steal(struct pipe_inode_info *, struct pipe_buffer *); | |
65 | +int generic_pipe_buf_nosteal(struct pipe_inode_info *, struct pipe_buffer *); | |
66 | void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *); | |
67 | void pipe_buf_mark_unmergeable(struct pipe_buffer *buf); | |
68 | ||
69 | --- a/kernel/trace/trace.c | |
70 | +++ b/kernel/trace/trace.c | |
71 | @@ -6823,19 +6823,23 @@ struct buffer_ref { | |
72 | struct ring_buffer *buffer; | |
73 | void *page; | |
74 | int cpu; | |
75 | - int ref; | |
76 | + refcount_t refcount; | |
77 | }; | |
78 | ||
79 | +static void buffer_ref_release(struct buffer_ref *ref) | |
80 | +{ | |
81 | + if (!refcount_dec_and_test(&ref->refcount)) | |
82 | + return; | |
83 | + ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page); | |
84 | + kfree(ref); | |
85 | +} | |
86 | + | |
87 | static void buffer_pipe_buf_release(struct pipe_inode_info *pipe, | |
88 | struct pipe_buffer *buf) | |
89 | { | |
90 | struct buffer_ref *ref = (struct buffer_ref *)buf->private; | |
91 | ||
92 | - if (--ref->ref) | |
93 | - return; | |
94 | - | |
95 | - ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page); | |
96 | - kfree(ref); | |
97 | + buffer_ref_release(ref); | |
98 | buf->private = 0; | |
99 | } | |
100 | ||
101 | @@ -6844,7 +6848,7 @@ static void buffer_pipe_buf_get(struct p | |
102 | { | |
103 | struct buffer_ref *ref = (struct buffer_ref *)buf->private; | |
104 | ||
105 | - ref->ref++; | |
106 | + refcount_inc(&ref->refcount); | |
107 | } | |
108 | ||
109 | /* Pipe buffer operations for a buffer. */ | |
110 | @@ -6852,7 +6856,7 @@ static const struct pipe_buf_operations | |
111 | .can_merge = 0, | |
112 | .confirm = generic_pipe_buf_confirm, | |
113 | .release = buffer_pipe_buf_release, | |
114 | - .steal = generic_pipe_buf_steal, | |
115 | + .steal = generic_pipe_buf_nosteal, | |
116 | .get = buffer_pipe_buf_get, | |
117 | }; | |
118 | ||
119 | @@ -6865,11 +6869,7 @@ static void buffer_spd_release(struct sp | |
120 | struct buffer_ref *ref = | |
121 | (struct buffer_ref *)spd->partial[i].private; | |
122 | ||
123 | - if (--ref->ref) | |
124 | - return; | |
125 | - | |
126 | - ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page); | |
127 | - kfree(ref); | |
128 | + buffer_ref_release(ref); | |
129 | spd->partial[i].private = 0; | |
130 | } | |
131 | ||
132 | @@ -6924,7 +6924,7 @@ tracing_buffers_splice_read(struct file | |
133 | break; | |
134 | } | |
135 | ||
136 | - ref->ref = 1; | |
137 | + refcount_set(&ref->refcount, 1); | |
138 | ref->buffer = iter->trace_buffer->buffer; | |
139 | ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file); | |
140 | if (IS_ERR(ref->page)) { |