Running the following command:
busybox cat /sys/kernel/debug/tracing/trace_pipe > /dev/null
with any tracing enabled pretty very quickly leads to various NULL
pointer dereferences and VM BUG_ON()s, such as these:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
IP: [<ffffffff8119df6c>] generic_pipe_buf_release+0xc/0x40
Call Trace:
[<ffffffff811c48a3>] splice_direct_to_actor+0x143/0x1e0
[<ffffffff811c42e0>] ? generic_pipe_buf_nosteal+0x10/0x10
[<ffffffff811c49cf>] do_splice_direct+0x8f/0xb0
[<ffffffff81196869>] do_sendfile+0x199/0x380
[<ffffffff81197600>] SyS_sendfile64+0x90/0xa0
[<ffffffff8192cbee>] entry_SYSCALL_64_fastpath+0x12/0x6d
page dumped because: VM_BUG_ON_PAGE(atomic_read(&page->_count) == 0)
kernel BUG at include/linux/mm.h:367!
invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
RIP: [<ffffffff8119df9c>] generic_pipe_buf_release+0x3c/0x40
Call Trace:
[<ffffffff811c48a3>] splice_direct_to_actor+0x143/0x1e0
[<ffffffff811c42e0>] ? generic_pipe_buf_nosteal+0x10/0x10
[<ffffffff811c49cf>] do_splice_direct+0x8f/0xb0
[<ffffffff81196869>] do_sendfile+0x199/0x380
[<ffffffff81197600>] SyS_sendfile64+0x90/0xa0
[<ffffffff8192cd1e>] tracesys_phase2+0x84/0x89
(busybox's cat uses sendfile(2), unlike the coreutils version)
This is because tracing_splice_read_pipe() can call splice_to_pipe()
with spd->nr_pages == 0. spd_pages underflows in splice_to_pipe() and
we fill the page pointers and the other fields of the pipe_buffers with
garbage.
All other callers of splice_to_pipe() avoid calling it when nr_pages ==
0, and we could make tracing_splice_read_pipe() do that too, but it
seems reasonable to have splice_to_page() handle this condition
gracefully.
Cc: [email protected]
Signed-off-by: Rabin Vincent <[email protected]>
---
fs/splice.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/splice.c b/fs/splice.c
index 82bc0d64fc38..19e0b103d253 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -185,6 +185,9 @@ ssize_t splice_to_pipe(struct pipe_inode_info *pipe,
unsigned int spd_pages = spd->nr_pages;
int ret, do_wakeup, page_nr;
+ if (!spd_pages)
+ return 0;
+
ret = 0;
do_wakeup = 0;
page_nr = 0;
--
2.7.0
Looks fine,
Reviewed-by: Christoph Hellwig <[email protected]>
On Thu, 10 Mar 2016 21:19:06 +0100
Rabin Vincent <[email protected]> wrote:
> Running the following command:
>
> busybox cat /sys/kernel/debug/tracing/trace_pipe > /dev/null
>
> with any tracing enabled pretty very quickly leads to various NULL
> pointer dereferences and VM BUG_ON()s, such as these:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
> IP: [<ffffffff8119df6c>] generic_pipe_buf_release+0xc/0x40
> Call Trace:
> [<ffffffff811c48a3>] splice_direct_to_actor+0x143/0x1e0
> [<ffffffff811c42e0>] ? generic_pipe_buf_nosteal+0x10/0x10
> [<ffffffff811c49cf>] do_splice_direct+0x8f/0xb0
> [<ffffffff81196869>] do_sendfile+0x199/0x380
> [<ffffffff81197600>] SyS_sendfile64+0x90/0xa0
> [<ffffffff8192cbee>] entry_SYSCALL_64_fastpath+0x12/0x6d
>
> page dumped because: VM_BUG_ON_PAGE(atomic_read(&page->_count) == 0)
> kernel BUG at include/linux/mm.h:367!
> invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> RIP: [<ffffffff8119df9c>] generic_pipe_buf_release+0x3c/0x40
> Call Trace:
> [<ffffffff811c48a3>] splice_direct_to_actor+0x143/0x1e0
> [<ffffffff811c42e0>] ? generic_pipe_buf_nosteal+0x10/0x10
> [<ffffffff811c49cf>] do_splice_direct+0x8f/0xb0
> [<ffffffff81196869>] do_sendfile+0x199/0x380
> [<ffffffff81197600>] SyS_sendfile64+0x90/0xa0
> [<ffffffff8192cd1e>] tracesys_phase2+0x84/0x89
>
> (busybox's cat uses sendfile(2), unlike the coreutils version)
Ouch!
>
> This is because tracing_splice_read_pipe() can call splice_to_pipe()
> with spd->nr_pages == 0. spd_pages underflows in splice_to_pipe() and
> we fill the page pointers and the other fields of the pipe_buffers with
> garbage.
>
> All other callers of splice_to_pipe() avoid calling it when nr_pages ==
> 0, and we could make tracing_splice_read_pipe() do that too, but it
> seems reasonable to have splice_to_page() handle this condition
> gracefully.
>
> Cc: [email protected]
> Signed-off-by: Rabin Vincent <[email protected]>
> ---
I think this should definitely be applied, but it's not my code to
apply it. I'll add this to make sure that trace_pipe is not the cause
anymore:
-- Steve
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0ae46048f724..cb2b708e4ea7 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4954,7 +4954,10 @@ static ssize_t tracing_splice_read_pipe(struct file *filp,
spd.nr_pages = i;
- ret = splice_to_pipe(pipe, &spd);
+ if (i)
+ ret = splice_to_pipe(pipe, &spd);
+ else
+ ret = 0;
out:
splice_shrink_spd(&spd);
return ret;
On Tue, 15 Mar 2016 00:23:20 -0700
Christoph Hellwig <[email protected]> wrote:
> Looks fine,
>
> Reviewed-by: Christoph Hellwig <[email protected]>
What tree should this go in?
-- Steve
On Fri, Mar 18, 2016 at 03:49:14PM -0400, Steven Rostedt wrote:
> On Tue, 15 Mar 2016 00:23:20 -0700
> Christoph Hellwig <[email protected]> wrote:
>
> > Looks fine,
> >
> > Reviewed-by: Christoph Hellwig <[email protected]>
>
> What tree should this go in?
Picked in vfs.git, will push to git.kernel.org shortly