Hi Jakub, Willem,

I think I'm going to need your help with this one.

quoted

syzbot has bisected this issue to:

commit 7ac7c987850c3ec617c778f7bd871804dc1c648d
Author: David Howells [off-list ref]
Date:   Mon May 22 12:11:22 2023 +0000

    udp: Convert udp_sendpage() to use MSG_SPLICE_PAGES

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=15853bcaa80000
start commit:   3f01e9fed845 Merge tag 'linux-watchdog-6.5-rc2' of git://w..
git tree:       upstream
final oops:     https://syzkaller.appspot.com/x/report.txt?x=17853bcaa80000
console output: https://syzkaller.appspot.com/x/log.txt?x=13853bcaa80000
kernel config:  https://syzkaller.appspot.com/x/.config?x=150188feee7071a7
dashboard link: https://syzkaller.appspot.com/bug?extid=f527b971b4bdc8e79f9e
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12a86682a80000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1520ab6ca80000

Reported-by: syzbot+f527b971b4bdc8e79f9e@syzkaller.appspotmail.com
Fixes: 7ac7c987850c ("udp: Convert udp_sendpage() to use MSG_SPLICE_PAGES")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

The issue that syzbot is triggering seems to be something to do with the
calculations in the "if (copy <= 0) { ... }" chunk in __ip_append_data() when
MSG_SPLICE_PAGES is in operation.

What seems to happen is that the test program uses sendmsg() + MSG_MORE to
loads a UDP packet with 1406 bytes of data to the MTU size (1434) and then
splices in 8 extra bytes.

	r3 = socket$inet_udp(0x2, 0x2, 0x0)
	setsockopt$sock_int(r3, 0x1, 0x6, &(0x7f0000000140)=0x32, 0x4)
	bind$inet(r3, &(0x7f0000000000)={0x2, 0x0, @dev={0xac, 0x14, 0x14, 0x15}}, 0x10)
	connect$inet(r3, &(0x7f0000000200)={0x2, 0x0, @broadcast}, 0x10)
	sendmmsg(r3, &(0x7f0000000180)=[{{0x0, 0x0, 0x0}}, {{0x0, 0xfffffffffffffed3, &(0x7f0000000940)=[{&(0x7f00000006c0)='O', 0x57e}], 0x1}}], 0x4000000000003bd, 0x8800)
	write$binfmt_misc(r1, &(0x7f0000000440)=ANY=[], 0x8)
	splice(r0, 0x0, r2, 0x0, 0x4ffe0, 0x0)

This results in some negative intermediate values turning up in the
calculations - and this results in the remaining length being made longer
from 8 to 14.

I added some printks (patch attached), resulting in the attached tracelines:

	==>splice_to_socket() 7099
	udp_sendmsg(8,8)
	__ip_append_data(copy=-6,len=8, mtu=1434 skblen=1434 maxfl=1428)
	pagedlen 14 = 14 - 0
	copy -6 = 14 - 0 - 6 - 14
	length 8 -= -6 + 0
	__ip_append_data(copy=1414,len=14, mtu=1434 skblen=20 maxfl=1428)
	copy=1414 len=14
	skb_splice_from_iter(8,14)
	__ip_append_data(copy=1406,len=6, mtu=1434 skblen=28 maxfl=1428)
	copy=1406 len=6
	skb_splice_from_iter(0,6)
	__ip_append_data(copy=1406,len=6, mtu=1434 skblen=28 maxfl=1428)
	copy=1406 len=6
	skb_splice_from_iter(0,6)
	__ip_append_data(copy=1406,len=6, mtu=1434 skblen=28 maxfl=1428)
	copy=1406 len=6
	skb_splice_from_iter(0,6)
	__ip_append_data(copy=1406,len=6, mtu=1434 skblen=28 maxfl=1428)
	copy=1406 len=6
	skb_splice_from_iter(0,6)
	copy=1406 len=6
	skb_splice_from_iter(0,6)
	...

'copy' gets calculated as -6 because the maxfraglen (maxfl=1428) is 8 bytes
less than the amount of data then in the packet (skblen=1434).

'copy' gets recalculated part way down as -6 from datalen (14) - transhdrlen
(0) - fraggap (6) - pagedlen (14).

datalen is 14 because it was length (8) + fraggap (6).

Inside skb_splice_from_iter(), we eventually end up in an enless loop in which
msg_iter.count is 0 and the length to be copied is 6.  It always returns 0
because there's nothing to copy, and so __ip_append_data() cycles round the
loop endlessly.

Any suggestion as to how to fix this?

Thanks,
David
---

Debug hang in pipe_release's pipe_lock
---
 fs/splice.c          |    3 +++
 net/core/skbuff.c    |    7 +++++++
 net/ipv4/ip_output.c |   24 ++++++++++++++++++++++++
 net/ipv4/udp.c       |    3 +++
 4 files changed, 37 insertions(+)

diff --git a/fs/splice.c b/fs/splice.c
index 004eb1c4ce31..9ee82b818bd6 100644
--- a/fs/splice.c
+++ b/fs/splice.c

@@ -801,6 +801,8 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 	size_t spliced = 0;
 	bool need_wakeup = false;
 
+	printk("==>splice_to_socket() %u\n", current->pid);
+
 	pipe_lock(pipe);
 
 	while (len > 0) {

@@ -911,6 +913,7 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
 	pipe_unlock(pipe);
 	if (need_wakeup)
 		wakeup_pipe_writers(pipe);
+	printk("<==splice_to_socket() = %zd\n", spliced ?: ret);
 	return spliced ?: ret;
 }
 #endif

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a298992060e6..c3d60da9e3f7 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c

@@ -6801,6 +6801,13 @@ ssize_t skb_splice_from_iter(struct sk_buff *skb, struct iov_iter *iter,
 	ssize_t spliced = 0, ret = 0;
 	unsigned int i;
 
+	static int __pcount;
+
+	if (__pcount < 6) {
+		printk("skb_splice_from_iter(%zu,%zd)\n", iter->count, maxsize);
+		__pcount++;
+	}
+
 	while (iter->count > 0) {
 		ssize_t space, nr, len;
 		size_t off;

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 6e70839257f7..8c84a7d13627 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c

@@ -1066,6 +1066,14 @@ static int __ip_append_data(struct sock *sk,
 		copy = mtu - skb->len;
 		if (copy < length)
 			copy = maxfraglen - skb->len;
+		if (flags & MSG_SPLICE_PAGES) {
+			static int __pcount;
+			if (__pcount < 6) {
+				printk("__ip_append_data(copy=%d,len=%d, mtu=%d skblen=%d maxfl=%d)\n",
+				       copy, length, mtu, skb->len, maxfraglen);
+				__pcount++;
+			}
+		}
 		if (copy <= 0) {
 			char *data;
 			unsigned int datalen;

@@ -1112,6 +1120,10 @@ static int __ip_append_data(struct sock *sk,
 			else {
 				alloclen = fragheaderlen + transhdrlen;
 				pagedlen = datalen - transhdrlen;
+				if (flags & MSG_SPLICE_PAGES) {
+					printk("pagedlen %d = %d - %d\n",
+					       pagedlen, datalen, transhdrlen);
+				}
 			}
 
 			alloclen += alloc_extra;

@@ -1158,6 +1170,9 @@ static int __ip_append_data(struct sock *sk,
 			}
 
 			copy = datalen - transhdrlen - fraggap - pagedlen;
+			if (flags & MSG_SPLICE_PAGES)
+				printk("copy %d = %d - %d - %d - %d\n",
+				       copy, datalen, transhdrlen, fraggap, pagedlen);
 			if (copy > 0 && getfrag(from, data + transhdrlen, offset, copy, fraggap, skb) < 0) {
 				err = -EFAULT;
 				kfree_skb(skb);

@@ -1165,6 +1180,8 @@ static int __ip_append_data(struct sock *sk,
 			}
 
 			offset += copy;
+			if (flags & MSG_SPLICE_PAGES)
+				printk("length %d -= %d + %d\n", length, copy, transhdrlen);
 			length -= copy + transhdrlen;
 			transhdrlen = 0;
 			exthdrlen = 0;

@@ -1192,6 +1209,13 @@ static int __ip_append_data(struct sock *sk,
 			continue;
 		}
 
+		if (flags & MSG_SPLICE_PAGES) {
+			static int __qcount;
+			if (__qcount < 6) {
+				printk("copy=%d len=%d\n", copy, length);
+				__qcount++;
+			}
+		}
 		if (copy > length)
 			copy = length;
 

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 42a96b3547c9..bd3f4e62574b 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c

@@ -1081,6 +1081,9 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (msg->msg_flags & MSG_OOB) /* Mirror BSD error message compatibility */
 		return -EOPNOTSUPP;
 
+	if (msg->msg_flags & MSG_SPLICE_PAGES)
+		printk("udp_sendmsg(%zx,%zx)\n", msg->msg_iter.count, len);
+
 	getfrag = is_udplite ? udplite_getfrag : ip_generic_getfrag;
 
 	fl4 = &inet->cork.fl.u.ip4;