linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] splice, net: Fix splice_to_socket() to handle pipe bufs larger than a page
@ 2023-06-14 10:09 David Howells
  2023-06-16  6:00 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 2+ messages in thread
From: David Howells @ 2023-06-14 10:09 UTC (permalink / raw)
  To: netdev
  Cc: dhowells, syzbot+f9e28a23426ac3b24f20, Willem de Bruijn,
	David Ahern, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Jens Axboe, Matthew Wilcox, Christian Brauner,
	Alexander Viro, linux-fsdevel, linux-kernel

    
splice_to_socket() assumes that a pipe_buffer won't hold more than a single
page of data - but this assumption can be violated by skb_splice_bits()
when it splices from a socket into a pipe.

The problem is that splice_to_socket() doesn't advance the pipe_buffer
length and offset when transcribing from the pipe buf into a bio_vec, so if
the buf is >PAGE_SIZE, it keeps repeating the same initial chunk and
doesn't advance the tail index.  It then subtracts this from "remain" and
overcounts the amount of data to be sent.

The cleanup phase then tries to overclean the pipe, hits an unused pipe buf
and a NULL-pointer dereference occurs.

Fix this by not restricting the bio_vec size to PAGE_SIZE and instead
transcribing the entirety of each pipe_buffer into a single bio_vec and
advancing the tail index if remain hasn't hit zero yet.

Large bio_vecs will then be split up by iterator functions such as
iov_iter_extract_pages().

This resulted in a KASAN report looking like:

general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
...
RIP: 0010:pipe_buf_release include/linux/pipe_fs_i.h:203 [inline]
RIP: 0010:splice_to_socket+0xa91/0xe30 fs/splice.c:933

Fixes: 2dc334f1a63a ("splice, net: Use sendmsg(MSG_SPLICE_PAGES) rather than ->sendpage()")
Reported-by: syzbot+f9e28a23426ac3b24f20@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/0000000000000900e905fdeb8e39@google.com/
Tested-by: syzbot+f9e28a23426ac3b24f20@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
cc: David Ahern <dsahern@kernel.org>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: Christian Brauner <brauner@kernel.org>
cc: Alexander Viro <viro@zeniv.linux.org.uk>
cc: netdev@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---
 fs/splice.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/splice.c b/fs/splice.c
index e337630aed64..567a1f03ea1e 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -886,7 +886,6 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 			}
 
 			seg = min_t(size_t, remain, buf->len);
-			seg = min_t(size_t, seg, PAGE_SIZE);
 
 			ret = pipe_buf_confirm(pipe, buf);
 			if (unlikely(ret)) {
@@ -897,10 +896,9 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 
 			bvec_set_page(&bvec[bc++], buf->page, seg, buf->offset);
 			remain -= seg;
-			if (seg >= buf->len)
-				tail++;
-			if (bc >= ARRAY_SIZE(bvec))
+			if (remain == 0 || bc >= ARRAY_SIZE(bvec))
 				break;
+			tail++;
 		}
 
 		if (!bc)


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net-next] splice, net: Fix splice_to_socket() to handle pipe bufs larger than a page
  2023-06-14 10:09 [PATCH net-next] splice, net: Fix splice_to_socket() to handle pipe bufs larger than a page David Howells
@ 2023-06-16  6:00 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-06-16  6:00 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, syzbot+f9e28a23426ac3b24f20, willemdebruijn.kernel,
	dsahern, davem, edumazet, kuba, pabeni, axboe, willy, brauner,
	viro, linux-fsdevel, linux-kernel

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 14 Jun 2023 11:09:48 +0100 you wrote:
> splice_to_socket() assumes that a pipe_buffer won't hold more than a single
> page of data - but this assumption can be violated by skb_splice_bits()
> when it splices from a socket into a pipe.
> 
> The problem is that splice_to_socket() doesn't advance the pipe_buffer
> length and offset when transcribing from the pipe buf into a bio_vec, so if
> the buf is >PAGE_SIZE, it keeps repeating the same initial chunk and
> doesn't advance the tail index.  It then subtracts this from "remain" and
> overcounts the amount of data to be sent.
> 
> [...]

Here is the summary with links:
  - [net-next] splice, net: Fix splice_to_socket() to handle pipe bufs larger than a page
    https://git.kernel.org/netdev/net-next/c/ca2d49f77ce4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-06-16  6:00 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-14 10:09 [PATCH net-next] splice, net: Fix splice_to_socket() to handle pipe bufs larger than a page David Howells
2023-06-16  6:00 ` patchwork-bot+netdevbpf

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).