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;
