[syzbot] WARNING in mptcp_sendmsg_frag

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: f306b90c69ce Merge tag 'smp-urgent-2021-09-12' of git://gi..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10694371300000
kernel config: https://syzkaller.appspot.com/x/.config?x=2bfb13fa4527da4e
dashboard link: https://syzkaller.appspot.com/bug?extid=263a248eec3e875baa7b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1

Unfortunately, I don't have any reproducer for this issue yet.

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

------------[ cut here ]------------
WARNING: CPU: 1 PID: 810 at net/mptcp/protocol.c:1366 mptcp_sendmsg_frag+0x1362/0x1bc0 net/mptcp/protocol.c:1366
Modules linked in:
CPU: 1 PID: 810 Comm: syz-executor.4 Not tainted 5.14.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:mptcp_sendmsg_frag+0x1362/0x1bc0 net/mptcp/protocol.c:1366
Code: ff 4c 8b 74 24 50 48 8b 5c 24 58 e9 0f fb ff ff e8 13 44 8b f8 4c 89 e7 45 31 ed e8 98 57 2e fe e9 81 f4 ff ff e8 fe 43 8b f8 <0f> 0b 41 bd ea ff ff ff e9 6f f4 ff ff 4c 89 e7 e8 b9 8e d2 f8 e9
RSP: 0018:ffffc9000531f6a0 EFLAGS: 00010216
RAX: 000000000000697f RBX: 0000000000000000 RCX: ffffc90012107000
RDX: 0000000000040000 RSI: ffffffff88eac9e2 RDI: 0000000000000003
RBP: ffff888078b15780 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff88eac017 R11: 0000000000000000 R12: ffff88801de0a280
R13: 0000000000006b58 R14: ffff888066278280 R15: ffff88803c2fe9c0
FS: 00007fd9f866e700(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007faebcb2f718 CR3: 00000000267cb000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
__mptcp_push_pending+0x1fb/0x6b0 net/mptcp/protocol.c:1547
mptcp_release_cb+0xfe/0x210 net/mptcp/protocol.c:3003
release_sock+0xb4/0x1b0 net/core/sock.c:3206
sk_stream_wait_memory+0x604/0xed0 net/core/stream.c:145
mptcp_sendmsg+0xc39/0x1bc0 net/mptcp/protocol.c:1749
inet6_sendmsg+0x99/0xe0 net/ipv6/af_inet6.c:643
sock_sendmsg_nosec net/socket.c:704 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:724
sock_write_iter+0x2a0/0x3e0 net/socket.c:1057
call_write_iter include/linux/fs.h:2163 [inline]
new_sync_write+0x40b/0x640 fs/read_write.c:507
vfs_write+0x7cf/0xae0 fs/read_write.c:594
ksys_write+0x1ee/0x250 fs/read_write.c:647
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x4665f9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 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 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fd9f866e188 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 000000000056c038 RCX: 00000000004665f9
RDX: 00000000000e7b78 RSI: 0000000020000000 RDI: 0000000000000003
RBP: 00000000004bfcc4 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000056c038
R13: 0000000000a9fb1f R14: 00007fd9f866e300 R15: 0000000000022000


---
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 syzk...@googlegroups.com.

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

[Paolo Abeni]

I think (mostly wild guess), this is caused by syzbot enabling tcp skb
tx recycling, so that in mptcp_sendmsg_frag() we end up with:

ssk->sk_tx_skb_cache != NULL

but:

skb_ext_find(ssk->sk_tx_skb_cache, SKB_EXT_MPTCP) == NULL.

Hard to say given the lack of reproducer. For -net we could do
something alike the following (some more testing needed), while for
net-next we have the sk_tx_skb_cache removal pending which should
address the issue.

/P
---
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 2602f1386160..f0673541a764 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1325,7 +1325,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
}

alloc_skb:
- if (!must_collapse && !ssk->sk_tx_skb_cache &&
+ if (!must_collapse &&
!mptcp_alloc_tx_skb(sk, ssk, info->data_lock_held))
return 0;

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit: 02319bf15acf net: dsa: bcm_sf2: Fix array overrun in bcm_s..
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=170f9e27300000
kernel config: https://syzkaller.appspot.com/x/.config?x=6d93fe4341f98704
dashboard link: https://syzkaller.appspot.com/bug?extid=263a248eec3e875baa7b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1507cd8d300000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=174c8017300000

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

------------[ cut here ]------------

WARNING: CPU: 0 PID: 7032 at net/mptcp/protocol.c:1366 mptcp_sendmsg_frag+0x1362/0x1bc0 net/mptcp/protocol.c:1366
Modules linked in:
CPU: 1 PID: 7032 Comm: syz-executor845 Not tainted 5.15.0-rc1-syzkaller #0

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:mptcp_sendmsg_frag+0x1362/0x1bc0 net/mptcp/protocol.c:1366

Code: ff 4c 8b 74 24 50 48 8b 5c 24 58 e9 0f fb ff ff e8 83 40 8b f8 4c 89 e7 45 31 ed e8 88 57 2e fe e9 81 f4 ff ff e8 6e 40 8b f8 <0f> 0b 41 bd ea ff ff ff e9 6f f4 ff ff 4c 89 e7 e8 b9 89 d2 f8 e9
RSP: 0018:ffffc90003acf830 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff888072918000 RSI: ffffffff88eacb72 RDI: 0000000000000003
RBP: ffff88807a182580 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff88eac1a7 R11: 0000000000000000 R12: ffff88801a08a000
R13: 0000000000000000 R14: ffff888018cb9b80 R15: ffff88801b4f2340
FS: 000055555723b300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000

CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

CR2: 0000000020000380 CR3: 000000007bebe000 CR4: 00000000001506e0

DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
__mptcp_push_pending+0x1fb/0x6b0 net/mptcp/protocol.c:1547

mptcp_sendmsg+0xc29/0x1bc0 net/mptcp/protocol.c:1748

inet6_sendmsg+0x99/0xe0 net/ipv6/af_inet6.c:643
sock_sendmsg_nosec net/socket.c:704 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:724
sock_write_iter+0x2a0/0x3e0 net/socket.c:1057
call_write_iter include/linux/fs.h:2163 [inline]
new_sync_write+0x40b/0x640 fs/read_write.c:507
vfs_write+0x7cf/0xae0 fs/read_write.c:594
ksys_write+0x1ee/0x250 fs/read_write.c:647
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

RIP: 0033:0x7f40ee3c4fb9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 15 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 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffd96b7a0f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f40ee3c4fb9

RDX: 00000000000e7b78 RSI: 0000000020000000 RDI: 0000000000000003

RBP: 0000000000000000 R08: 0000000000f0b5ff R09: 0000000000f0b5ff
R10: 0000000000000004 R11: 0000000000000246 R12: 0000000000012096
R13: 00007ffd96b7a120 R14: 00007ffd96b7a110 R15: 00007ffd96b7a104

[Paolo Abeni]

On Tue, 2021-09-14 at 21:05 -0700, syzbot wrote:

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

tentative -net fix

[syzbot]

Hello,

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

------------[ cut here ]------------
WARNING: CPU: 0 PID: 7555 at net/mptcp/protocol.c:1366 mptcp_sendmsg_frag+0x16e1/0x1b90 net/mptcp/protocol.c:1366
Modules linked in:
CPU: 0 PID: 7555 Comm: syz-executor840 Not tainted 5.15.0-rc2-syzkaller #0

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

RIP: 0010:mptcp_sendmsg_frag+0x16e1/0x1b90 net/mptcp/protocol.c:1366
Code: f8 4c 89 ef 45 31 f6 e8 ed 57 2e fe e9 ad f2 ff ff e8 a3 3b 8b f8 89 de 4c 89 ef e8 99 b9 ff ff e9 a4 fc ff ff e8 8f 3b 8b f8 <0f> 0b 41 be ea ff ff ff e9 87 f2 ff ff e8 7d 3b 8b f8 31 d2 44 89
RSP: 0018:ffffc9000432f850 EFLAGS: 00010293

RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000

RDX: ffff888079549c80 RSI: ffffffff88ead031 RDI: 0000000000000003
RBP: ffff888025f78c80 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff88eac5de R11: 0000000000000000 R12: ffff88801ac03b80
R13: ffff88801ac03900 R14: 0000000000000000 R15: ffff88807bca8000
FS: 00005555572f0300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000

CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

CR2: 00007ffed96ab948 CR3: 0000000074a2a000 CR4: 00000000001506e0

DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
__mptcp_push_pending+0x1fb/0x6b0 net/mptcp/protocol.c:1547

mptcp_sendmsg+0xc29/0x1bc0 net/mptcp/protocol.c:1748

inet6_sendmsg+0x99/0xe0 net/ipv6/af_inet6.c:643
sock_sendmsg_nosec net/socket.c:704 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:724

sock_write_iter+0x289/0x3c0 net/socket.c:1057
call_write_iter include/linux/fs.h:2163 [inline]
new_sync_write+0x429/0x660 fs/read_write.c:507

vfs_write+0x7cf/0xae0 fs/read_write.c:594
ksys_write+0x1ee/0x250 fs/read_write.c:647
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

RIP: 0033:0x7f9576e96fa9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 15 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 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffed96ab968 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f9576e96fa9

RDX: 00000000000e7b78 RSI: 0000000020000000 RDI: 0000000000000003

RBP: 0000000000000000 R08: 0000000000f0b5ff R09: 0000000000f0b5ff

R10: 0000000000000004 R11: 0000000000000246 R12: 000000000001a761
R13: 00007ffed96ab990 R14: 00007ffed96ab980 R15: 00007ffed96ab974


Tested on:

commit: 4c17ca27 Merge tag 'spi-fix-v5.15-rc2' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17c699ab300000
kernel config: https://syzkaller.appspot.com/x/.config?x=e917f3dfc452c977
dashboard link: https://syzkaller.appspot.com/bug?extid=263a248eec3e875baa7b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
patch: https://syzkaller.appspot.com/x/patch.diff?x=115e8cad300000

[Paolo Abeni]

On Tue, 2021-09-14 at 21:05 -0700, syzbot wrote:

#syz test: https://github.com/multipath-tcp/mptcp_net-next.git export

checking if the current export branch is still prone to the issue

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+263a24...@syzkaller.appspotmail.com

Tested on:

commit: 92d5e767 DO-NOT-MERGE: mptcp: enabled by default
git tree: https://github.com/multipath-tcp/mptcp_net-next.git export
kernel config: https://syzkaller.appspot.com/x/.config?x=6d93fe4341f98704
dashboard link: https://syzkaller.appspot.com/bug?extid=263a248eec3e875baa7b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

Note: testing is done by a robot and is best-effort only.

[Paolo Abeni]

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

no clue why we still hit the warn, and can't reproduce the issue
locally.

Add some debug code.

---
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c

index 2602f1386160..0d20279024fb 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1295,6 +1295,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
int size_bias = 0;
int avail_size;
size_t ret = 0;
+ int mfrags;

pr_debug("msk=%p ssk=%p sending dfrag at seq=%llu len=%u already sent=%u",
msk, ssk, dfrag->data_seq, dfrag->data_len, info->sent);
@@ -1302,6 +1303,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
/* compute send limit */
info->mss_now = tcp_send_mss(ssk, &info->size_goal, info->flags);
avail_size = info->size_goal;
+ mfrags = READ_ONCE(sysctl_max_skb_frags);
skb = tcp_write_queue_tail(ssk);
if (skb) {
/* Limit the write to the size available in the
@@ -1317,7 +1319,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
}

must_collapse = (info->size_goal - skb->len > 0) &&
- (skb_shinfo(skb)->nr_frags < sysctl_max_skb_frags);
+ (skb_shinfo(skb)->nr_frags < mfrags);
if (must_collapse) {
size_bias = skb->len;
avail_size = info->size_goal - skb->len;
@@ -1325,7 +1327,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,

}

alloc_skb:
- if (!must_collapse && !ssk->sk_tx_skb_cache &&
+ if (!must_collapse &&
!mptcp_alloc_tx_skb(sk, ssk, info->data_lock_held))
return 0;

@@ -1363,6 +1365,11 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
}

mpext = skb_ext_find(tail, SKB_EXT_MPTCP);
+ if (!mpext)
+ pr_warn("must_collapse=%d old skb=%p avail_size=%d zero_window_probe=%d ws=%d state=%d frags=%d:%d",
+ must_collapse, skb, avail_size, zero_window_probe,
+ mptcp_subflow_ctx(ssk)->rel_write_seq,
+ ssk->sk_state, mfrags, READ_ONCE(sysctl_max_skb_frags));
if (WARN_ON_ONCE(!mpext)) {
/* should never reach here, stream corrupted */
return -EINVAL;

[syzbot]

Hello,

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

MPTCP: must_collapse=1 old skb=ffff88807de82a00 avail_size=-376 zero_window_probe=0 ws=32729 state=1 frags=17:17
------------[ cut here ]------------
WARNING: CPU: 0 PID: 7472 at net/mptcp/protocol.c:1373 mptcp_sendmsg_frag.cold+0xe0/0x113 net/mptcp/protocol.c:1369
Modules linked in:
CPU: 1 PID: 7472 Comm: syz-executor252 Not tainted 5.15.0-rc2-syzkaller #0

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

RIP: 0010:mptcp_sendmsg_frag.cold+0xe0/0x113 net/mptcp/protocol.c:1373
Code: 4d 38 53 41 0f b6 f4 4c 89 fa 8b 44 24 54 48 c7 c7 c0 6e b6 8a 50 41 55 44 8b 44 24 70 8b 4c 24 2c 41 83 e0 01 e8 e3 77 be ff <0f> 0b 41 bc ea ff ff ff 48 83 c4 18 e9 5e 5d b6 ff 48 c7 c7 18 b3
RSP: 0018:ffffc9000375f838 EFLAGS: 00010286
RAX: 0000000000000070 RBX: 0000000000000011 RCX: 0000000000000000
RDX: ffff8880760ab900 RSI: ffffffff815dbd98 RDI: fffff520006ebef9
RBP: ffff888016660600 R08: 0000000000000070 R09: 0000000000000000
R10: ffffffff815d5b3e R11: 0000000000000000 R12: 0000000000000001
R13: 0000000000000001 R14: 0000000000000000 R15: ffff88807de82a00
FS: 00005555563a6300(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000

CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

CR2: 00007f065b9f3000 CR3: 000000007db04000 CR4: 00000000001506e0

DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:

__mptcp_push_pending+0x1fb/0x6b0 net/mptcp/protocol.c:1554
mptcp_sendmsg+0xc29/0x1bc0 net/mptcp/protocol.c:1755

inet6_sendmsg+0x99/0xe0 net/ipv6/af_inet6.c:643
sock_sendmsg_nosec net/socket.c:704 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:724

sock_write_iter+0x289/0x3c0 net/socket.c:1057
call_write_iter include/linux/fs.h:2163 [inline]
new_sync_write+0x429/0x660 fs/read_write.c:507

vfs_write+0x7cf/0xae0 fs/read_write.c:594
ksys_write+0x1ee/0x250 fs/read_write.c:647
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

RIP: 0033:0x7f0a4ab2efa9

Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 41 15 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 c0 ff ff ff f7 d8 64 89 01 48

RSP: 002b:00007ffee3a1d078 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f0a4ab2efa9

RDX: 00000000000e7b78 RSI: 0000000020000000 RDI: 0000000000000003

RBP: 0000000000000000 R08: 0000000900f0b5ff R09: 0000000900f0b5ff
R10: 0000000000000004 R11: 0000000000000246 R12: 000000000001a80f
R13: 00007ffee3a1d0a0 R14: 00007ffee3a1d090 R15: 00007ffee3a1d084


Tested on:

commit: 92477dd1 Merge tag 's390-5.15-ebpf-jit-fixes' of git:/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13c58a4b300000
kernel config: https://syzkaller.appspot.com/x/.config?x=e917f3dfc452c977

dashboard link: https://syzkaller.appspot.com/bug?extid=263a248eec3e875baa7b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=14a55423300000

[Paolo Abeni]

On Sat, 2021-09-18 at 04:50 -0700, syzbot wrote:

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

The debug code helped a bit. It looks like we have singed/unsigned
comparisons issue

Tentative patch, plus debug code

---
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c

index 2602f1386160..c38506c5ea05 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1316,7 +1316,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
goto alloc_skb;
}

- must_collapse = (info->size_goal - skb->len > 0) &&
+ must_collapse = (info->size_goal > skb->len) &&
(skb_shinfo(skb)->nr_frags < sysctl_max_skb_frags);

if (must_collapse) {
size_bias = skb->len;

@@ -1325,7 +1325,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,

}

alloc_skb:
- if (!must_collapse && !ssk->sk_tx_skb_cache &&
+ if (!must_collapse &&
!mptcp_alloc_tx_skb(sk, ssk, info->data_lock_held))
return 0;

@@ -1363,6 +1363,10 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,

}

mpext = skb_ext_find(tail, SKB_EXT_MPTCP);
+ if (!mpext)

+ pr_warn("must_collapse=%d old skb=%p:%d avail_size=%d:%d state=%d",
+ must_collapse, skb, skb ? skb->len:0, info->size_goal, avail_size,
+ ssk->sk_state);

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+263a24...@syzkaller.appspotmail.com

Tested on:

commit: 92477dd1 Merge tag 's390-5.15-ebpf-jit-fixes' of git:/..
git tree: upstream

kernel config: https://syzkaller.appspot.com/x/.config?x=e917f3dfc452c977

dashboard link: https://syzkaller.appspot.com/bug?extid=263a248eec3e875baa7b
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=142df1ab300000

[Dan Carpenter]

On Wed, Sep 22, 2021 at 12:32:56PM +0200, Paolo Abeni wrote:
>
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>
> The debug code helped a bit. It looks like we have singed/unsigned
> comparisons issue

There should be a static checker warning for these. I have created one
in response to your email. It turns out there are a couple other
instances of this bug in the same file.

net/mptcp/protocol.c:479 mptcp_subflow_could_cleanup() warn: unsigned subtraction: '(null)' use '!='
net/mptcp/protocol.c:909 mptcp_frag_can_collapse_to() warn: unsigned subtraction: 'pfrag->size - pfrag->offset' use '!='
net/mptcp/protocol.c:1319 mptcp_sendmsg_frag() warn: unsigned subtraction: 'info->size_goal - skb->len' use '!='

regards,
dan carpenter

[Dan Carpenter]

On Thu, Sep 23, 2021 at 05:19:42PM +0300, Dan Carpenter wrote:
> On Wed, Sep 22, 2021 at 12:32:56PM +0200, Paolo Abeni wrote:
> >
> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >
> > The debug code helped a bit. It looks like we have singed/unsigned
> > comparisons issue
>
> There should be a static checker warning for these. I have created one
> in response to your email. It turns out there are a couple other
> instances of this bug in the same file.
>
> net/mptcp/protocol.c:479 mptcp_subflow_could_cleanup() warn: unsigned subtraction: '(null)' use '!='

I should have checked my output a bit more carefully. I don't want this
one to generate a warning.

> net/mptcp/protocol.c:909 mptcp_frag_can_collapse_to() warn: unsigned subtraction: 'pfrag->size - pfrag->offset' use '!='

Likely "pfrag->offset" can't be larger than "pfrag->size". Smatch has
some code to try track this information but it's not clever enough.

regards,
dan carpenter

[Paolo Abeni]

Hello,

On Thu, 2021-09-23 at 17:37 +0300, Dan Carpenter wrote:
> On Thu, Sep 23, 2021 at 05:19:42PM +0300, Dan Carpenter wrote:
> > On Wed, Sep 22, 2021 at 12:32:56PM +0200, Paolo Abeni wrote:
> > > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > >
> > > The debug code helped a bit. It looks like we have singed/unsigned
> > > comparisons issue
> >
> > There should be a static checker warning for these. I have created one
> > in response to your email. It turns out there are a couple other
> > instances of this bug in the same file.

Thank you!

I was quite suprised the plain compiler did not emit a warn, even with
W=1.

> > net/mptcp/protocol.c:479 mptcp_subflow_could_cleanup() warn: unsigned subtraction: '(null)' use '!='
>
> I should have checked my output a bit more carefully. I don't want this
> one to generate a warning.
>
> > net/mptcp/protocol.c:909 mptcp_frag_can_collapse_to() warn: unsigned subtraction: 'pfrag->size - pfrag->offset' use '!='
>
> Likely "pfrag->offset" can't be larger than "pfrag->size". Smatch has
> some code to try track this information but it's not clever enough.

Yes, this looks safe, offset can't be larger than size.

Even the last reported warning looks safe to me: 'info->size_goal -
skb->len', we just check for size_goal being greater then skb->len.

Cheers,

Paolo