Re: WARNING in sctp_assoc_update_frag_point

[Xin Long]

On Tue, Jun 26, 2018 at 1:06 PM, syzbot
<syzbot+f0d9d7cba052f9344b03@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    6f0d349d922b Merge git://git.kernel.org/pub/scm/linux/kern..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12a423c0400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=a63be0c83e84d370
> dashboard link: https://syzkaller.appspot.com/bug?extid=f0d9d7cba052f9344b03
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+f0d9d7cba052f9344b03@xxxxxxxxxxxxxxxxxxxxxxxxx
>
> WARNING: CPU: 0 PID: 22543 at include/net/sctp/sctp.h:598 sctp_mtu_payload
> include/net/sctp/sctp.h:598 [inline]
> WARNING: CPU: 0 PID: 22543 at include/net/sctp/sctp.h:598
> sctp_assoc_update_frag_point+0x252/0x2c0 net/sctp/associola.c:1401
> Kernel panic - not syncing: panic_on_warn set ...
>
> CPU: 0 PID: 22543 Comm: syz-executor2 Not tainted 4.18.0-rc2+ #117
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
>  panic+0x238/0x4e7 kernel/panic.c:184
>  __warn.cold.8+0x163/0x1ba kernel/panic.c:536
>  report_bug+0x252/0x2d0 lib/bug.c:186
>  fixup_bug arch/x86/kernel/traps.c:178 [inline]
>  do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296
>  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316
>  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
> RIP: 0010:sctp_mtu_payload include/net/sctp/sctp.h:598 [inline]
> RIP: 0010:sctp_assoc_update_frag_point+0x252/0x2c0 net/sctp/associola.c:1401
> Code: 76 fa 45 39 e5 76 1e e8 0c 69 76 fa 45 29 e5 45 89 ec e9 34 ff ff ff
> e8 fc 68 76 fa 45 8d 66 34 e9 09 ff ff ff e8 ee 68 76 fa <0f> 0b 45 31 e4 e9
> 17 ff ff ff e8 7f 3c b4 fa e9 31 fe ff ff 4c 89
> RSP: 0018:ffff8801d7def378 EFLAGS: 00010216
> RAX: 0000000000040000 RBX: ffff8801d8580ac0 RCX: ffffc900133ca000
> RDX: 00000000000001b9 RSI: ffffffff8705a382 RDI: 0000000000000004
> RBP: ffff8801d7def3a0 R08: ffff8801cfaa6000 R09: ffffed002e0421af
> R10: ffffed002e0421af R11: ffff880170210d7f R12: 0000000000000044
> R13: 0000000000000044 R14: 0000000000000010 R15: ffff8801d8580ac0
>  sctp_assoc_set_pmtu net/sctp/associola.c:1417 [inline]
>  sctp_assoc_sync_pmtu+0x251/0x2e0 net/sctp/associola.c:1445
We may need a fix:
@@ -282,7 +282,10 @@ bool sctp_transport_update_pmtu(struct
sctp_transport *t, u32 pmtu)

        if (dst) {
                /* Re-fetch, as under layers may have a higher minimum size */
-               pmtu = SCTP_TRUNC4(dst_mtu(dst));
+               int dst_mtu = SCTP_TRUNC4(dst_mtu(dst));
+
+               if (pmtu < dst_mtu)
+                       pmtu = dst_mtu;
                change = t->pathmtu != pmtu;

to make sure the t->pathmtu never get a value smaller than MINSEGMENT,
also meets the comments:
"Re-fetch, as under layers may have a higher minimum size".