diff mbox

splice: handle zero nr_pages in splice_to_pipe()

Message ID 1457641146-9068-1-git-send-email-rabin@rab.in (mailing list archive)
State New, archived
Headers show

Commit Message

Rabin Vincent March 10, 2016, 8:19 p.m. UTC
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: stable@vger.kernel.org
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 fs/splice.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Christoph Hellwig March 15, 2016, 7:23 a.m. UTC | #1
Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steven Rostedt March 18, 2016, 7:49 p.m. UTC | #2
On Tue, 15 Mar 2016 00:23:20 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> Looks fine,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

What tree should this go in?

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Al Viro March 18, 2016, 8:05 p.m. UTC | #3
On Fri, Mar 18, 2016 at 03:49:14PM -0400, Steven Rostedt wrote:
> On Tue, 15 Mar 2016 00:23:20 -0700
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > Looks fine,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> What tree should this go in?

Picked in vfs.git, will push to git.kernel.org shortly
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;