* [for-linus][PATCH 0/3] tracing: Minor fixes that will be going to Linus shortly
@ 2019-04-26 15:46 Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 1/3] tracing: Fix buffer_ref pipe ops Steven Rostedt
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-04-26 15:46 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
ftrace/urgent
Head SHA1: d6097c9e4454adf1f8f2c9547c2fa6060d55d952
Jann Horn (1):
tracing: Fix buffer_ref pipe ops
Peter Zijlstra (1):
trace: Fix preempt_enable_no_resched() abuse
Wenwen Wang (1):
tracing: Fix a memory leak by early error exit in trace_pid_write()
----
fs/splice.c | 4 ++--
include/linux/pipe_fs_i.h | 1 +
kernel/trace/ring_buffer.c | 2 +-
kernel/trace/trace.c | 33 ++++++++++++++++++---------------
4 files changed, 22 insertions(+), 18 deletions(-)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [for-linus][PATCH 1/3] tracing: Fix buffer_ref pipe ops
2019-04-26 15:46 [for-linus][PATCH 0/3] tracing: Minor fixes that will be going to Linus shortly Steven Rostedt
@ 2019-04-26 15:46 ` Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 2/3] tracing: Fix a memory leak by early error exit in trace_pid_write() Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 3/3] trace: Fix preempt_enable_no_resched() abuse Steven Rostedt
2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-04-26 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Ingo Molnar, Masami Hiramatsu,
Al Viro, stable, Jann Horn
From: Jann Horn <jannh@google.com>
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>
---
fs/splice.c | 4 ++--
include/linux/pipe_fs_i.h | 1 +
kernel/trace/trace.c | 28 ++++++++++++++--------------
3 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/fs/splice.c b/fs/splice.c
index 3ee7e82df48f..e75807380caa 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -330,8 +330,8 @@ const struct pipe_buf_operations default_pipe_buf_ops = {
.get = generic_pipe_buf_get,
};
-static int generic_pipe_buf_nosteal(struct pipe_inode_info *pipe,
- struct pipe_buffer *buf)
+int generic_pipe_buf_nosteal(struct pipe_inode_info *pipe,
+ struct pipe_buffer *buf)
{
return 1;
}
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 787d224ff43e..a830e9a00eb9 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -174,6 +174,7 @@ void free_pipe_info(struct pipe_inode_info *);
void generic_pipe_buf_get(struct pipe_inode_info *, struct pipe_buffer *);
int generic_pipe_buf_confirm(struct pipe_inode_info *, struct pipe_buffer *);
int generic_pipe_buf_steal(struct pipe_inode_info *, struct pipe_buffer *);
+int generic_pipe_buf_nosteal(struct pipe_inode_info *, struct pipe_buffer *);
void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *);
void pipe_buf_mark_unmergeable(struct pipe_buffer *buf);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 21153e64bf1c..0cfa13a60086 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7025,19 +7025,23 @@ struct buffer_ref {
struct ring_buffer *buffer;
void *page;
int cpu;
- int ref;
+ refcount_t refcount;
};
+static void buffer_ref_release(struct buffer_ref *ref)
+{
+ if (!refcount_dec_and_test(&ref->refcount))
+ return;
+ ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page);
+ kfree(ref);
+}
+
static void buffer_pipe_buf_release(struct pipe_inode_info *pipe,
struct pipe_buffer *buf)
{
struct buffer_ref *ref = (struct buffer_ref *)buf->private;
- if (--ref->ref)
- return;
-
- ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page);
- kfree(ref);
+ buffer_ref_release(ref);
buf->private = 0;
}
@@ -7046,14 +7050,14 @@ static void buffer_pipe_buf_get(struct pipe_inode_info *pipe,
{
struct buffer_ref *ref = (struct buffer_ref *)buf->private;
- ref->ref++;
+ refcount_inc(&ref->refcount);
}
/* Pipe buffer operations for a buffer. */
static const struct pipe_buf_operations buffer_pipe_buf_ops = {
.confirm = generic_pipe_buf_confirm,
.release = buffer_pipe_buf_release,
- .steal = generic_pipe_buf_steal,
+ .steal = generic_pipe_buf_nosteal,
.get = buffer_pipe_buf_get,
};
@@ -7066,11 +7070,7 @@ static void buffer_spd_release(struct splice_pipe_desc *spd, unsigned int i)
struct buffer_ref *ref =
(struct buffer_ref *)spd->partial[i].private;
- if (--ref->ref)
- return;
-
- ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page);
- kfree(ref);
+ buffer_ref_release(ref);
spd->partial[i].private = 0;
}
@@ -7125,7 +7125,7 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
break;
}
- ref->ref = 1;
+ refcount_set(&ref->refcount, 1);
ref->buffer = iter->trace_buffer->buffer;
ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file);
if (IS_ERR(ref->page)) {
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [for-linus][PATCH 2/3] tracing: Fix a memory leak by early error exit in trace_pid_write()
2019-04-26 15:46 [for-linus][PATCH 0/3] tracing: Minor fixes that will be going to Linus shortly Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 1/3] tracing: Fix buffer_ref pipe ops Steven Rostedt
@ 2019-04-26 15:46 ` Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 3/3] trace: Fix preempt_enable_no_resched() abuse Steven Rostedt
2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-04-26 15:46 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, stable, Wenwen Wang
From: Wenwen Wang <wang6495@umn.edu>
In trace_pid_write(), the buffer for trace parser is allocated through
kmalloc() in trace_parser_get_init(). Later on, after the buffer is used,
it is then freed through kfree() in trace_parser_put(). However, it is
possible that trace_pid_write() is terminated due to unexpected errors,
e.g., ENOMEM. In that case, the allocated buffer will not be freed, which
is a memory leak bug.
To fix this issue, free the allocated buffer when an error is encountered.
Link: http://lkml.kernel.org/r/1555726979-15633-1-git-send-email-wang6495@umn.edu
Fixes: f4d34a87e9c10 ("tracing: Use pid bitmap instead of a pid array for set_event_pid")
Cc: stable@vger.kernel.org
Signed-off-by: Wenwen Wang <wang6495@umn.edu>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0cfa13a60086..46f68fad6373 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -496,8 +496,10 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
* not modified.
*/
pid_list = kmalloc(sizeof(*pid_list), GFP_KERNEL);
- if (!pid_list)
+ if (!pid_list) {
+ trace_parser_put(&parser);
return -ENOMEM;
+ }
pid_list->pid_max = READ_ONCE(pid_max);
@@ -507,6 +509,7 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
pid_list->pids = vzalloc((pid_list->pid_max + 7) >> 3);
if (!pid_list->pids) {
+ trace_parser_put(&parser);
kfree(pid_list);
return -ENOMEM;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [for-linus][PATCH 3/3] trace: Fix preempt_enable_no_resched() abuse
2019-04-26 15:46 [for-linus][PATCH 0/3] tracing: Minor fixes that will be going to Linus shortly Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 1/3] tracing: Fix buffer_ref pipe ops Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 2/3] tracing: Fix a memory leak by early error exit in trace_pid_write() Steven Rostedt
@ 2019-04-26 15:46 ` Steven Rostedt
2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-04-26 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Waiman Long, Linus Torvalds,
Ingo Molnar, Will Deacon, Thomas Gleixner,
the arch/x86 maintainers, Davidlohr Bueso, Tim Chen, huang ying,
Roman Gushchin, Alexei Starovoitov, Daniel Borkmann, stable,
Peter Zijlstra (Intel)
From: Peter Zijlstra <peterz@infradead.org>
Unless the very next line is schedule(), or implies it, one must not use
preempt_enable_no_resched(). It can cause a preemption to go missing and
thereby cause arbitrary delays, breaking the PREEMPT=y invariant.
Link: http://lkml.kernel.org/r/20190423200318.GY14281@hirez.programming.kicks-ass.net
Cc: Waiman Long <longman@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: the arch/x86 maintainers <x86@kernel.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: huang ying <huang.ying.caritas@gmail.com>
Cc: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: stable@vger.kernel.org
Fixes: 2c2d7329d8af ("tracing/ftrace: use preempt_enable_no_resched_notrace in ring_buffer_time_stamp()")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ring_buffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 41b6f96e5366..4ee8d8aa3d0f 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -762,7 +762,7 @@ u64 ring_buffer_time_stamp(struct ring_buffer *buffer, int cpu)
preempt_disable_notrace();
time = rb_time_stamp(buffer);
- preempt_enable_no_resched_notrace();
+ preempt_enable_notrace();
return time;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-26 15:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-26 15:46 [for-linus][PATCH 0/3] tracing: Minor fixes that will be going to Linus shortly Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 1/3] tracing: Fix buffer_ref pipe ops Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 2/3] tracing: Fix a memory leak by early error exit in trace_pid_write() Steven Rostedt
2019-04-26 15:46 ` [for-linus][PATCH 3/3] trace: Fix preempt_enable_no_resched() abuse Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).