Endless loop in udp with MSG_SPLICE_READ - Re: [syzbot] [fs?] INFO: task hung in pipe_release (4)
From: David Howells
Date: Sat Jul 29 2023 - 17:50:31 EST
Hi Jakub, Willem,
I think I'm going to need your help with this one.
> > syzbot has bisected this issue to:
> >
> > commit 7ac7c987850c3ec617c778f7bd871804dc1c648d
> > Author: David Howells <dhowells@xxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > 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;