[syzbot] [net?] WARNING in __ip6_append_data

[syzbot] [net?] WARNING in __ip6_append_data

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    65d6e954e378 Merge tag 'gfs2-v6.5-rc5-fixes' of git://git...
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=142177f4680000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b273cdfbc13e9a4b
dashboard link: https://syzkaller.appspot.com/bug?extid=62cbf263225ae13ff153
compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11f37a0c680000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10034f3fa80000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/74df7181e630/disk-65d6e954.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/8455d5988dfe/vmlinux-65d6e954.xz
kernel image: https://storage.googleapis.com/syzbot-assets/8ee7b79f0dfd/bzImage-65d6e954.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+62cbf263225ae13ff153@syzkaller.appspotmail.com

------------[ cut here ]------------
WARNING: CPU: 1 PID: 5042 at net/ipv6/ip6_output.c:1800 __ip6_append_data.isra.0+0x1be8/0x47f0 net/ipv6/ip6_output.c:1800
Modules linked in:
CPU: 1 PID: 5042 Comm: syz-executor133 Not tainted 6.5.0-syzkaller-11938-g65d6e954e378 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/26/2023
RIP: 0010:__ip6_append_data.isra.0+0x1be8/0x47f0 net/ipv6/ip6_output.c:1800
Code: db f6 ff ff e8 09 d5 97 f8 49 8d 44 24 ff 48 89 44 24 60 49 8d 6c 24 07 e9 c2 f6 ff ff 4c 8b b4 24 90 01 00 00 e8 e8 d4 97 f8 <0f> 0b 48 8b 44 24 10 45 89 f4 48 8d 98 74 02 00 00 e8 d2 d4 97 f8
RSP: 0018:ffffc90003a1f3b8 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000001004 RCX: 0000000000000000
RDX: ffff88801fe70000 RSI: ffffffff88efcf18 RDI: 0000000000000006
RBP: 0000000000001000 R08: 0000000000000006 R09: 0000000000001004
R10: 0000000000001000 R11: 0000000000000000 R12: 0000000000000001
R13: dffffc0000000000 R14: 0000000000001004 R15: ffff888019f31000
FS:  0000555557280380(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000045ad50 CR3: 0000000072666000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 ip6_append_data+0x1e6/0x510 net/ipv6/ip6_output.c:1895
 l2tp_ip6_sendmsg+0xdf9/0x1cc0 net/l2tp/l2tp_ip6.c:631
 inet_sendmsg+0x9d/0xe0 net/ipv4/af_inet.c:840
 sock_sendmsg_nosec net/socket.c:730 [inline]
 sock_sendmsg+0xd9/0x180 net/socket.c:753
 splice_to_socket+0xade/0x1010 fs/splice.c:881
 do_splice_from fs/splice.c:933 [inline]
 direct_splice_actor+0x118/0x180 fs/splice.c:1142
 splice_direct_to_actor+0x347/0xa30 fs/splice.c:1088
 do_splice_direct+0x1af/0x280 fs/splice.c:1194
 do_sendfile+0xb88/0x1390 fs/read_write.c:1254
 __do_sys_sendfile64 fs/read_write.c:1322 [inline]
 __se_sys_sendfile64 fs/read_write.c:1308 [inline]
 __x64_sys_sendfile64+0x1d6/0x220 fs/read_write.c:1308
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f6b11150469
Code: 48 83 c4 28 c3 e8 37 17 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fffd14e71a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000028
RAX: ffffffffffffffda RBX: 00007fffd14e7378 RCX: 00007f6b11150469
RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000005
RBP: 00007f6b111c3610 R08: 00007fffd14e7378 R09: 00007fffd14e7378
R10: 000000010000a006 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fffd14e7368 R14: 0000000000000001 R15: 0000000000000001
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [net?] WARNING in __ip6_append_data

[Eric Dumazet]

On Wed, Sep 13, 2023 at 8:19 AM syzbot
<syzbot+62cbf263225ae13ff153@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    65d6e954e378 Merge tag 'gfs2-v6.5-rc5-fixes' of git://git...
> git tree:       upstream
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=142177f4680000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=b273cdfbc13e9a4b
> dashboard link: https://syzkaller.appspot.com/bug?extid=62cbf263225ae13ff153
> compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11f37a0c680000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10034f3fa80000
>

CC David

Warning added in

commit ce650a1663354a6cad7145e7f5131008458b39d4
Author: David Howells <dhowells@redhat.com>
Date:   Wed Aug 2 08:36:50 2023 +0100

    udp6: Fix __ip6_append_data()'s handling of MSG_SPLICE_PAGES


> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/74df7181e630/disk-65d6e954.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/8455d5988dfe/vmlinux-65d6e954.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/8ee7b79f0dfd/bzImage-65d6e954.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+62cbf263225ae13ff153@syzkaller.appspotmail.com
>
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 5042 at net/ipv6/ip6_output.c:1800 __ip6_append_data.isra.0+0x1be8/0x47f0 net/ipv6/ip6_output.c:1800
> Modules linked in:
> CPU: 1 PID: 5042 Comm: syz-executor133 Not tainted 6.5.0-syzkaller-11938-g65d6e954e378 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/26/2023
> RIP: 0010:__ip6_append_data.isra.0+0x1be8/0x47f0 net/ipv6/ip6_output.c:1800
> Code: db f6 ff ff e8 09 d5 97 f8 49 8d 44 24 ff 48 89 44 24 60 49 8d 6c 24 07 e9 c2 f6 ff ff 4c 8b b4 24 90 01 00 00 e8 e8 d4 97 f8 <0f> 0b 48 8b 44 24 10 45 89 f4 48 8d 98 74 02 00 00 e8 d2 d4 97 f8
> RSP: 0018:ffffc90003a1f3b8 EFLAGS: 00010293
> RAX: 0000000000000000 RBX: 0000000000001004 RCX: 0000000000000000
> RDX: ffff88801fe70000 RSI: ffffffff88efcf18 RDI: 0000000000000006
> RBP: 0000000000001000 R08: 0000000000000006 R09: 0000000000001004
> R10: 0000000000001000 R11: 0000000000000000 R12: 0000000000000001
> R13: dffffc0000000000 R14: 0000000000001004 R15: ffff888019f31000
> FS:  0000555557280380(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000000000045ad50 CR3: 0000000072666000 CR4: 00000000003506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  <TASK>
>  ip6_append_data+0x1e6/0x510 net/ipv6/ip6_output.c:1895
>  l2tp_ip6_sendmsg+0xdf9/0x1cc0 net/l2tp/l2tp_ip6.c:631
>  inet_sendmsg+0x9d/0xe0 net/ipv4/af_inet.c:840
>  sock_sendmsg_nosec net/socket.c:730 [inline]
>  sock_sendmsg+0xd9/0x180 net/socket.c:753
>  splice_to_socket+0xade/0x1010 fs/splice.c:881
>  do_splice_from fs/splice.c:933 [inline]
>  direct_splice_actor+0x118/0x180 fs/splice.c:1142
>  splice_direct_to_actor+0x347/0xa30 fs/splice.c:1088
>  do_splice_direct+0x1af/0x280 fs/splice.c:1194
>  do_sendfile+0xb88/0x1390 fs/read_write.c:1254
>  __do_sys_sendfile64 fs/read_write.c:1322 [inline]
>  __se_sys_sendfile64 fs/read_write.c:1308 [inline]
>  __x64_sys_sendfile64+0x1d6/0x220 fs/read_write.c:1308
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7f6b11150469
> Code: 48 83 c4 28 c3 e8 37 17 00 00 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007fffd14e71a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000028
> RAX: ffffffffffffffda RBX: 00007fffd14e7378 RCX: 00007f6b11150469
> RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000005
> RBP: 00007f6b111c3610 R08: 00007fffd14e7378 R09: 00007fffd14e7378
> R10: 000000010000a006 R11: 0000000000000246 R12: 0000000000000001
> R13: 00007fffd14e7368 R14: 0000000000000001 R15: 0000000000000001
>  </TASK>
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the bug is already fixed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite bug's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the bug is a duplicate of another bug, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup

Re: [syzbot] [net?] WARNING in __ip6_append_data

[David Howells]

Hi Eric,

> > WARNING: CPU: 1 PID: 5042 at net/ipv6/ip6_output.c:1800 __ip6_append_data.isra.0+0x1be8/0x47f0 net/ipv6/ip6_output.c:1800

That would appear to be this:

			if (WARN_ON_ONCE(copy > msg->msg_iter.count))
				goto error;

However, I have a problem that the repro program errors out at this point
before it gets that far:

	if (cork->length + length > maxnonfragsize - headersize) {
   emsgsize:
		pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
		ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
		return -EMSGSIZE;
	}

Are you able to reproduce the issue?

The values in and around that point are:

	cork->length		0
	length			65540
	maxnonfragsize		65575
	headersize		40
	transhdrlen		4
	mtu			65536
	ip6_sk_ignore_df(sk)	true

with maxnonfragsize coming from 'sizeof(struct ipv6hdr) + IPV6_MAXPLEN'.  Is
that even viable for the size of a packet?

David

Re: [syzbot] [net?] WARNING in __ip6_append_data

[David Howells]

I think the attached is probably an equivalent cleaned up reproducer.  Note
that if the length given to sendfile() is less than 65536, it fails with
EINVAL before it gets into __ip6_append_data().

David
---
#define _GNU_SOURCE
#include <arpa/inet.h>
#include <fcntl.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/sendfile.h>
#include <sys/uio.h>
#include <netinet/ip6.h>
#include <netinet/in.h>

#define IPPROTO_L2TP 115

#define OSERROR(R, S) \
	do { if ((long)(R) == -1L) { perror((S)); exit(1); } } while(0)

static char buffer[16776960];

int main()
{
	struct sockaddr_in6 sin6;
	int ffd, dfd, sfd;

	ffd = openat(AT_FDCWD, "cgroup.controllers",
		     O_RDWR | O_CREAT | O_TRUNC, 0);
	OSERROR(ffd, "cgroup.controllers/f");

	OSERROR(write(ffd, buffer, sizeof(buffer)), "write");

	dfd = openat(AT_FDCWD, "cgroup.controllers",
		     O_RDONLY | O_NONBLOCK | O_DIRECT);
	OSERROR(dfd, "cgroup.controllers/d");

	sfd = socket(AF_INET6, SOCK_DGRAM, IPPROTO_L2TP);
	OSERROR(dfd, "socket");

	memset(&sin6, 0, sizeof(sin6));
	sin6.sin6_family = AF_INET6;
	sin6.sin6_port = htons(0);
	sin6.sin6_addr.s6_addr32[0] = htonl(0);
	sin6.sin6_addr.s6_addr32[1] = htonl(0);
	sin6.sin6_addr.s6_addr32[2] = htonl(0);
	sin6.sin6_addr.s6_addr32[3] = htonl(1);
	OSERROR(bind(sfd, (struct sockaddr *)&sin6, 32),
		"bind");

	memset(&sin6, 0, sizeof(sin6));
	sin6.sin6_family = AF_INET6;
	sin6.sin6_port = htons(7);
	sin6.sin6_addr.s6_addr32[0] = htonl(0);
	sin6.sin6_addr.s6_addr32[1] = htonl(0);
	sin6.sin6_addr.s6_addr32[2] = htonl(0);
	sin6.sin6_addr.s6_addr32[3] = htonl(1);
	OSERROR(connect(sfd, (struct sockaddr *)&sin6, 32),
		"connect");

	OSERROR(sendfile(sfd, dfd, NULL, 65536), "sendfile");
	return 0;
}

Re: [syzbot] [net?] WARNING in __ip6_append_data

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> I think the attached is probably an equivalent cleaned up reproducer.  Note
> that if the length given to sendfile() is less than 65536, it fails with
> EINVAL before it gets into __ip6_append_data().

Actually, it only fails with EINVAL if the size is not a multiple of the block
size of the source file because it's open O_DIRECT so, say, 65536-512 is fine
(and works).

But thinking more on this further, is this even a bug in my code, I wonder?
The length passed is 65536 - but a UDP packet can't carry that, so it
shouldn't it have errored out before getting that far?  (which is what it
seems to do when I try it).

I don't see how we get past the length check in ip6_append_data() with the
reproducer we're given unless the MTU is somewhat bigger than 65536 (is that
even possible?)

David

Re: [syzbot] [net?] WARNING in __ip6_append_data

[David Howells]

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 65d6e954e378

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 0665e8b09968..7978335c1fc4 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1517,8 +1517,10 @@ static int __ip6_append_data(struct sock *sk,
 		     rt->rt6i_nfheader_len;
 
 	if (mtu <= fragheaderlen ||
-	    ((mtu - fragheaderlen) & ~7) + fragheaderlen <= sizeof(struct frag_hdr))
+	    ((mtu - fragheaderlen) & ~7) + fragheaderlen <= sizeof(struct frag_hdr)) {
+		printk("__%u__: EMSGSIZE\n", __LINE__);
 		goto emsgsize;
+	}
 
 	maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen -
 		     sizeof(struct frag_hdr);
@@ -1526,8 +1528,10 @@ static int __ip6_append_data(struct sock *sk,
 	/* as per RFC 7112 section 5, the entire IPv6 Header Chain must fit
 	 * the first fragment
 	 */
-	if (headersize + transhdrlen > mtu)
+	if (headersize + transhdrlen > mtu) {
+		printk("__%u__: EMSGSIZE\n", __LINE__);
 		goto emsgsize;
+	}
 
 	if (cork->length + length > mtu - headersize && ipc6->dontfrag &&
 	    (sk->sk_protocol == IPPROTO_UDP ||
@@ -1535,15 +1539,23 @@ static int __ip6_append_data(struct sock *sk,
 	     sk->sk_protocol == IPPROTO_RAW)) {
 		ipv6_local_rxpmtu(sk, fl6, mtu - headersize +
 				sizeof(struct ipv6hdr));
+		printk("__%u__: EMSGSIZE\n", __LINE__);
 		goto emsgsize;
 	}
 
-	if (ip6_sk_ignore_df(sk))
+	if (ip6_sk_ignore_df(sk)) {
 		maxnonfragsize = sizeof(struct ipv6hdr) + IPV6_MAXPLEN;
-	else
+		printk("MAXPLEN\n");
+	} else {
 		maxnonfragsize = mtu;
+		printk("mtu\n");
+	}
 
+	printk("check %d %zd %d %d, %d %d\n",
+	       cork->length, length, maxnonfragsize, headersize,
+	       transhdrlen, mtu);
 	if (cork->length + length > maxnonfragsize - headersize) {
+		printk("__%u__: EMSGSIZE\n", __LINE__);
 emsgsize:
 		pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
 		ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
@@ -1817,8 +1829,10 @@ static int __ip6_append_data(struct sock *sk,
 			if (!skb_can_coalesce(skb, i, pfrag->page,
 					      pfrag->offset)) {
 				err = -EMSGSIZE;
-				if (i == MAX_SKB_FRAGS)
+				if (i == MAX_SKB_FRAGS) {
+					printk("__%u__: EMSGSIZE\n", __LINE__);
 					goto error;
+				}
 
 				__skb_fill_page_desc(skb, i, pfrag->page,
 						     pfrag->offset, 0);
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index ed8ebb6f5909..daaaf60dce01 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -502,6 +502,8 @@ static int l2tp_ip6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int ulen;
 	int err;
 
+	printk("%s()\n", __func__);
+
 	/* Rough check on arithmetic overflow,
 	 * better check is made in ip6_append_data().
 	 */

Re: [syzbot] [net?] WARNING in __ip6_append_data

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING in __ip6_append_data

l2tp_ip6_sendmsg()
MAXPLEN
check 0 4100 65575 40, 4 65536
l2tp_ip6_sendmsg()
MAXPLEN
check 4100 4100 65575 40, 0 65536
------------[ cut here ]------------
WARNING: CPU: 0 PID: 5455 at net/ipv6/ip6_output.c:1812 __ip6_append_data.isra.0+0x1c6d/0x4900 net/ipv6/ip6_output.c:1812
Modules linked in:
CPU: 0 PID: 5455 Comm: syz-executor.0 Not tainted 6.5.0-syzkaller-11938-g65d6e954e378-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/04/2023
RIP: 0010:__ip6_append_data.isra.0+0x1c6d/0x4900 net/ipv6/ip6_output.c:1812
Code: c4 f6 ff ff e8 84 d4 97 f8 49 8d 44 24 ff 48 89 44 24 68 49 8d 6c 24 07 e9 ab f6 ff ff 4c 8b b4 24 90 01 00 00 e8 63 d4 97 f8 <0f> 0b 48 8b 44 24 10 45 89 f4 48 8d 98 74 02 00 00 e8 4d d4 97 f8
RSP: 0018:ffffc90004f373b8 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000001004 RCX: 0000000000000000
RDX: ffff888019e8bb80 RSI: ffffffff88efcf9d RDI: 0000000000000006
RBP: 0000000000001000 R08: 0000000000000006 R09: 0000000000001004
R10: 0000000000001000 R11: 0000000000000001 R12: 0000000000000001
R13: dffffc0000000000 R14: 0000000000001004 R15: ffff888027b1d640
FS:  00007feae40ff6c0(0000) GS:ffff8880b9800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f0f01e4e378 CR3: 000000007d467000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 ip6_append_data+0x1e6/0x510 net/ipv6/ip6_output.c:1909
 l2tp_ip6_sendmsg+0xe0c/0x1ce0 net/l2tp/l2tp_ip6.c:633
 inet_sendmsg+0x9d/0xe0 net/ipv4/af_inet.c:840
 sock_sendmsg_nosec net/socket.c:730 [inline]
 sock_sendmsg+0xd9/0x180 net/socket.c:753
 splice_to_socket+0xade/0x1010 fs/splice.c:881
 do_splice_from fs/splice.c:933 [inline]
 direct_splice_actor+0x118/0x180 fs/splice.c:1142
 splice_direct_to_actor+0x347/0xa30 fs/splice.c:1088
 do_splice_direct+0x1af/0x280 fs/splice.c:1194
 do_sendfile+0xb88/0x1390 fs/read_write.c:1254
 __do_sys_sendfile64 fs/read_write.c:1322 [inline]
 __se_sys_sendfile64 fs/read_write.c:1308 [inline]
 __x64_sys_sendfile64+0x1d6/0x220 fs/read_write.c:1308
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7feae347cae9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007feae40ff0c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000028
RAX: ffffffffffffffda RBX: 00007feae359bf80 RCX: 00007feae347cae9
RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000005
RBP: 00007feae34c847a R08: 0000000000000000 R09: 0000000000000000
R10: 000000010000a006 R11: 0000000000000246 R12: 0000000000000000
R13: 000000000000000b R14: 00007feae359bf80 R15: 00007ffc444d03c8
 </TASK>


Tested on:

commit:         65d6e954 Merge tag 'gfs2-v6.5-rc5-fixes' of git://git...
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=12133ac4680000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b273cdfbc13e9a4b
dashboard link: https://syzkaller.appspot.com/bug?extid=62cbf263225ae13ff153
compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=139cae54680000

Re: [syzbot] [net?] WARNING in __ip6_append_data

[Willem de Bruijn]

David Howells wrote:
> David Howells <dhowells@redhat.com> wrote:
> 
> > I think the attached is probably an equivalent cleaned up reproducer.  Note
> > that if the length given to sendfile() is less than 65536, it fails with
> > EINVAL before it gets into __ip6_append_data().
> 
> Actually, it only fails with EINVAL if the size is not a multiple of the block
> size of the source file because it's open O_DIRECT so, say, 65536-512 is fine
> (and works).
> 
> But thinking more on this further, is this even a bug in my code, I wonder?
> The length passed is 65536 - but a UDP packet can't carry that, so it
> shouldn't it have errored out before getting that far?  (which is what it
> seems to do when I try it).
> 
> I don't see how we get past the length check in ip6_append_data() with the
> reproducer we're given unless the MTU is somewhat bigger than 65536 (is that
> even possible?)

An ipv6 packet can carry 64KB of payload, so maxnonfragsize of 65535 + 40
sounds correct. But payload length passed of 65536 is not (ignoring ipv6
jumbograms). So that should probably trigger an EINVAL -- if that is indeed
what the repro does.

Re: [syzbot] [net?] WARNING in __ip6_append_data

[Eric Dumazet]

On Mon, Sep 18, 2023 at 3:58 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> David Howells wrote:
> > David Howells <dhowells@redhat.com> wrote:
> >
> > > I think the attached is probably an equivalent cleaned up reproducer.  Note
> > > that if the length given to sendfile() is less than 65536, it fails with
> > > EINVAL before it gets into __ip6_append_data().
> >
> > Actually, it only fails with EINVAL if the size is not a multiple of the block
> > size of the source file because it's open O_DIRECT so, say, 65536-512 is fine
> > (and works).
> >
> > But thinking more on this further, is this even a bug in my code, I wonder?
> > The length passed is 65536 - but a UDP packet can't carry that, so it
> > shouldn't it have errored out before getting that far?  (which is what it
> > seems to do when I try it).
> >
> > I don't see how we get past the length check in ip6_append_data() with the
> > reproducer we're given unless the MTU is somewhat bigger than 65536 (is that
> > even possible?)
>
> An ipv6 packet can carry 64KB of payload, so maxnonfragsize of 65535 + 40
> sounds correct. But payload length passed of 65536 is not (ignoring ipv6
> jumbograms). So that should probably trigger an EINVAL -- if that is indeed
> what the repro does.

l2tp_ip6_sendmsg() claims ip6_append_data() can make better checks,
but what about simply replacing INT_MAX by 65535 ?

diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 44cfb72bbd18a34e83e50bebca09729c55df524f..ab57a134923bfc8040dba0d8fb702551ff265184
100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -502,10 +502,7 @@ static int l2tp_ip6_sendmsg(struct sock *sk,
struct msghdr *msg, size_t len)
        int ulen;
        int err;

-       /* Rough check on arithmetic overflow,
-        * better check is made in ip6_append_data().
-        */
-       if (len > INT_MAX - transhdrlen)
+       if (len > 65535 - transhdrlen)
                return -EMSGSIZE;
        ulen = len + transhdrlen;

Re: [syzbot] [net?] WARNING in __ip6_append_data

[David Howells]

Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> 
> An ipv6 packet can carry 64KB of payload, so maxnonfragsize of 65535 + 40
> sounds correct. But payload length passed of 65536 is not (ignoring ipv6
> jumbograms). So that should probably trigger an EINVAL -- if that is indeed
> what the repro does.

The problem is that on entry to __ip6_append_data(), the length includes
transhdrlen.  However, this is a problem if we already have something in the
packet.  At that point, this fails:

			if (WARN_ON_ONCE(copy > msg->msg_iter.count))
				goto error;

because copy includes transhdrlen.

David

Re: [syzbot] [net?] WARNING in __ip6_append_data

[Tom Parkin]

[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]

On  Mon, Sep 18, 2023 at 16:04:49 +0200, Eric Dumazet wrote:
> On Mon, Sep 18, 2023 at 3:58 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > David Howells wrote:
> > > David Howells <dhowells@redhat.com> wrote:
> > >
> > > > I think the attached is probably an equivalent cleaned up reproducer.  Note
> > > > that if the length given to sendfile() is less than 65536, it fails with
> > > > EINVAL before it gets into __ip6_append_data().
> > >
> > > Actually, it only fails with EINVAL if the size is not a multiple of the block
> > > size of the source file because it's open O_DIRECT so, say, 65536-512 is fine
> > > (and works).
> > >
> > > But thinking more on this further, is this even a bug in my code, I wonder?
> > > The length passed is 65536 - but a UDP packet can't carry that, so it
> > > shouldn't it have errored out before getting that far?  (which is what it
> > > seems to do when I try it).
> > >
> > > I don't see how we get past the length check in ip6_append_data() with the
> > > reproducer we're given unless the MTU is somewhat bigger than 65536 (is that
> > > even possible?)
> >
> > An ipv6 packet can carry 64KB of payload, so maxnonfragsize of 65535 + 40
> > sounds correct. But payload length passed of 65536 is not (ignoring ipv6
> > jumbograms). So that should probably trigger an EINVAL -- if that is indeed
> > what the repro does.
> 
> l2tp_ip6_sendmsg() claims ip6_append_data() can make better checks,
> but what about simply replacing INT_MAX by 65535 ?

Slightly OT but I think the l2tp_ip6.c approach was probably cribbed
from net/ipv6/udp.c's udpv6_sendmsg originally:


    /* Rough check on arithmetic overflow,
       better check is made in ip6_append_data().
       */
    if (len > INT_MAX - sizeof(struct udphdr))
        return -EMSGSIZE;


Should the udp code be modified similarly?

-- 
Tom Parkin
Katalix Systems Ltd
https://katalix.com
Catalysts for your Embedded Linux software development

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [syzbot] [net?] WARNING in __ip6_append_data

[Eric Dumazet]

On Tue, Sep 19, 2023 at 10:27 AM Tom Parkin <tparkin@katalix.com> wrote:
>
> On  Mon, Sep 18, 2023 at 16:04:49 +0200, Eric Dumazet wrote:
> > On Mon, Sep 18, 2023 at 3:58 PM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > David Howells wrote:
> > > > David Howells <dhowells@redhat.com> wrote:
> > > >
> > > > > I think the attached is probably an equivalent cleaned up reproducer.  Note
> > > > > that if the length given to sendfile() is less than 65536, it fails with
> > > > > EINVAL before it gets into __ip6_append_data().
> > > >
> > > > Actually, it only fails with EINVAL if the size is not a multiple of the block
> > > > size of the source file because it's open O_DIRECT so, say, 65536-512 is fine
> > > > (and works).
> > > >
> > > > But thinking more on this further, is this even a bug in my code, I wonder?
> > > > The length passed is 65536 - but a UDP packet can't carry that, so it
> > > > shouldn't it have errored out before getting that far?  (which is what it
> > > > seems to do when I try it).
> > > >
> > > > I don't see how we get past the length check in ip6_append_data() with the
> > > > reproducer we're given unless the MTU is somewhat bigger than 65536 (is that
> > > > even possible?)
> > >
> > > An ipv6 packet can carry 64KB of payload, so maxnonfragsize of 65535 + 40
> > > sounds correct. But payload length passed of 65536 is not (ignoring ipv6
> > > jumbograms). So that should probably trigger an EINVAL -- if that is indeed
> > > what the repro does.
> >
> > l2tp_ip6_sendmsg() claims ip6_append_data() can make better checks,
> > but what about simply replacing INT_MAX by 65535 ?
>
> Slightly OT but I think the l2tp_ip6.c approach was probably cribbed
> from net/ipv6/udp.c's udpv6_sendmsg originally:
>
>
>     /* Rough check on arithmetic overflow,
>        better check is made in ip6_append_data().
>        */
>     if (len > INT_MAX - sizeof(struct udphdr))
>         return -EMSGSIZE;
>
>
> Should the udp code be modified similarly?
>

Unfortunately both l2tp and udp support CORK (MSG_MORE),
so a modified check like that will not be enough to prevent syzbot reports.

Better than nothing, of course.

I also note that ipv4 size of l2tp does not have any check,
an overflow seems possible with carefully chosen size.