Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From:   David Howells <dhowells@redhat.com>
In-Reply-To: <0000000000000900e905fdeb8e39@google.com>
References: <0000000000000900e905fdeb8e39@google.com>
To:     syzbot <syzbot+f9e28a23426ac3b24f20@syzkaller.appspotmail.com>
Cc:     dhowells@redhat.com, brauner@kernel.org, kuba@kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, syzkaller-bugs@googlegroups.com,
        viro@zeniv.linux.org.uk
Subject: Re: [syzbot] [fs?] general protection fault in splice_to_socket
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1423847.1686733230.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date:   Wed, 14 Jun 2023 10:00:30 +0100
Message-ID: <1423848.1686733230@warthog.procyon.org.uk>
Precedence: bulk

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.g=
it main

commit d302bc9baf84c549891bedee57ee917d9e0485d7
Author: David Howells <dhowells@redhat.com>
Date:   Wed Jun 14 09:14:50 2023 +0100

    splice: Fix splice_to_socket() to handle pipe bufs larger than a page
    =

    splice_to_socket() assumes that a pipe_buffer won't hold more than a s=
ingle
    page of data - but it seems that this assumption can be violated when
    splicing 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 pip=
e 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 an=
d
    advancing the tail index if remain hasn't hit zero yet.
    =

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

    This resulted in a KASAN report looking like:
    =

    general protection fault, probably for non-canonical address 0xdffffc0=
000000001: 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
    =

    Reported-by: syzbot+f9e28a23426ac3b24f20@syzkaller.appspotmail.com
    Fixes: 2dc334f1a63a ("splice, net: Use sendmsg(MSG_SPLICE_PAGES) rathe=
r than ->sendpage()")
    =

    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

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 =3D min_t(size_t, remain, buf->len);
-			seg =3D min_t(size_t, seg, PAGE_SIZE);
 =

 			ret =3D 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 -=3D seg;
-			if (seg >=3D buf->len)
-				tail++;
-			if (bc >=3D ARRAY_SIZE(bvec))
+			if (remain =3D=3D 0 || bc >=3D ARRAY_SIZE(bvec))
 				break;
+			tail++;
 		}
 =

 		if (!bc)