Commit 92fdd98c authored by Al Viro's avatar Al Viro Committed by Steven Rostedt

tracing: Fix buggered tee(2) on tracing_pipe

In kernel/trace/trace.c we have this:
static void tracing_pipe_buf_release(struct pipe_inode_info *pipe,
                                     struct pipe_buffer *buf)
{
        __free_page(buf->page);
}
static const struct pipe_buf_operations tracing_pipe_buf_ops = {
        .can_merge              = 0,
        .map                    = generic_pipe_buf_map,
        .unmap                  = generic_pipe_buf_unmap,
        .confirm                = generic_pipe_buf_confirm,
        .release                = tracing_pipe_buf_release,
        .steal                  = generic_pipe_buf_steal,
        .get                    = generic_pipe_buf_get,
};
with
void generic_pipe_buf_get(struct pipe_inode_info *pipe, struct pipe_buffer *buf)
{
        page_cache_get(buf->page);
}

and I don't see anything that would've prevented tee(2) called on the pipe
that got stuff spliced into it from that sucker.  ->ops->get() will be
called, then buf gets copied into target pipe's ->bufs[] and eventually
readers get to both copies of the buffer.  With
	get_page(page)
	look at that page
	__free_page(page)
	look at that page
	__free_page(page)
which is not a good thing, to put it mildly.  AFAICS, that ought to use
the normal generic_pipe_buf_release() (aka page_cache_release(buf->page)),
shouldn't it?

[
 SDR - As trace_pipe just allocates the page with alloc_page(GFP_KERNEL),
  and doesn't do anything special with it (no LRU logic). The __free_page()
  should be fine, as it wont actually free a page with reference count.
  Maybe there's a chance to leak memory? Anyway, This change is at a minimum
  good for being symmetric with generic_pipe_buf_get, it is fine to add.
]
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
[ SDR - Removed no longer used tracing_pipe_buf_release ]
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
parent dced341b
...@@ -4243,12 +4243,6 @@ tracing_read_pipe(struct file *filp, char __user *ubuf, ...@@ -4243,12 +4243,6 @@ tracing_read_pipe(struct file *filp, char __user *ubuf,
return sret; return sret;
} }
static void tracing_pipe_buf_release(struct pipe_inode_info *pipe,
struct pipe_buffer *buf)
{
__free_page(buf->page);
}
static void tracing_spd_release_pipe(struct splice_pipe_desc *spd, static void tracing_spd_release_pipe(struct splice_pipe_desc *spd,
unsigned int idx) unsigned int idx)
{ {
...@@ -4260,7 +4254,7 @@ static const struct pipe_buf_operations tracing_pipe_buf_ops = { ...@@ -4260,7 +4254,7 @@ static const struct pipe_buf_operations tracing_pipe_buf_ops = {
.map = generic_pipe_buf_map, .map = generic_pipe_buf_map,
.unmap = generic_pipe_buf_unmap, .unmap = generic_pipe_buf_unmap,
.confirm = generic_pipe_buf_confirm, .confirm = generic_pipe_buf_confirm,
.release = tracing_pipe_buf_release, .release = generic_pipe_buf_release,
.steal = generic_pipe_buf_steal, .steal = generic_pipe_buf_steal,
.get = generic_pipe_buf_get, .get = generic_pipe_buf_get,
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment