2019-05-28 00:49:56

by syzbot

[permalink] [raw]
Subject: memory leak in sctp_process_init

Hello,

syzbot found the following crash on:

HEAD commit: 9c7db500 Merge tag 'selinux-pr-20190521' of git://git.kern..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10388530a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=61dd9e15a761691d
dashboard link: https://syzkaller.appspot.com/bug?extid=f7e9153b037eac9b1df8
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10e32f8ca00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=177fa530a00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]

0 to HW filter on device batadv0
executing program
executing program
executing program
BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.770s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301

BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.840s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301

BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.910s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301

BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.980s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301

BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 29.050s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301

BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 29.120s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301

BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 29.190s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301

BUG: memory leak
unreferenced object 0xffff88810ef68400 (size 1024):
comm "syz-executor273", pid 7046, jiffies 4294945598 (age 29.260s)
hex dump (first 32 bytes):
1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000a02cebbd>] kmemleak_alloc_recursive
include/linux/kmemleak.h:55 [inline]
[<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
[<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
[<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
[<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
[<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
[<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
net/sctp/sm_make_chunk.c:2437
[<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
[inline]
[<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
[inline]
[<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
[inline]
[<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
net/sctp/sm_sideeffect.c:1165
[<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200
net/sctp/associola.c:1074
[<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95
[<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354
[<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline]
[<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418
[<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934
[<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122
[<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802
[<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292
[<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330
[<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline]
[<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline]
[<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337
[<00000000a8b4131f>] do_syscall_64+0x76/0x1a0
arch/x86/entry/common.c:301



---
This bug 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 [email protected].

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches


2019-05-28 01:39:05

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Mon, May 27, 2019 at 05:48:06PM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 9c7db500 Merge tag 'selinux-pr-20190521' of git://git.kern..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=10388530a00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=61dd9e15a761691d
> dashboard link: https://syzkaller.appspot.com/bug?extid=f7e9153b037eac9b1df8
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10e32f8ca00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=177fa530a00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: [email protected]
>
> 0 to HW filter on device batadv0
> executing program
> executing program
> executing program
> BUG: memory leak
> unreferenced object 0xffff88810ef68400 (size 1024):
> comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.770s)
> hex dump (first 32 bytes):
> 1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<00000000a02cebbd>] kmemleak_alloc_recursive
> include/linux/kmemleak.h:55 [inline]
> [<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
> [<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
> [<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
> [<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
> [<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
> [<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
> [<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
> net/sctp/sm_make_chunk.c:2437
> [<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
> [inline]
> [<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
> [inline]
> [<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
> [inline]
> [<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
> net/sctp/sm_sideeffect.c:1165

Note that this is on the client side. It was handling the INIT_ACK
chunk, from sctp_sf_do_5_1C_ack().

I'm not seeing anything else other than sctp_association_free()
releasing this memory. This means 2 things:
- Every time the cookie is retransmitted, it leaks. As shown by the
repetitive leaks here.
- The cookie remains allocated throughout the association, which is
also not good as that's a 1k that we could have released back to the
system right after the handshake.

Marcelo

2019-05-28 11:18:20

by Neil Horman

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Mon, May 27, 2019 at 10:36:00PM -0300, Marcelo Ricardo Leitner wrote:
> On Mon, May 27, 2019 at 05:48:06PM -0700, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 9c7db500 Merge tag 'selinux-pr-20190521' of git://git.kern..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=10388530a00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=61dd9e15a761691d
> > dashboard link: https://syzkaller.appspot.com/bug?extid=f7e9153b037eac9b1df8
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10e32f8ca00000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=177fa530a00000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: [email protected]
> >
> > 0 to HW filter on device batadv0
> > executing program
> > executing program
> > executing program
> > BUG: memory leak
> > unreferenced object 0xffff88810ef68400 (size 1024):
> > comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.770s)
> > hex dump (first 32 bytes):
> > 1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > backtrace:
> > [<00000000a02cebbd>] kmemleak_alloc_recursive
> > include/linux/kmemleak.h:55 [inline]
> > [<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
> > [<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
> > [<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
> > [<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
> > [<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
> > [<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
> > [<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
> > net/sctp/sm_make_chunk.c:2437
> > [<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
> > [inline]
> > [<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
> > [inline]
> > [<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
> > [inline]
> > [<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
> > net/sctp/sm_sideeffect.c:1165
>
> Note that this is on the client side. It was handling the INIT_ACK
> chunk, from sctp_sf_do_5_1C_ack().
>
> I'm not seeing anything else other than sctp_association_free()
> releasing this memory. This means 2 things:
> - Every time the cookie is retransmitted, it leaks. As shown by the
> repetitive leaks here.
> - The cookie remains allocated throughout the association, which is
> also not good as that's a 1k that we could have released back to the
> system right after the handshake.
>
> Marcelo
>
If we have an INIT chunk bundled with a COOKIE_ECHO chunk in the same packet,
this might occur. Processing for each chunk (via sctp_cmd_process_init and
sctp_sf_do_5_1D_ce both call sctp_process_init, which would cause a second write
to asoc->peer.cookie, leaving the first write (set via kmemdup), to be orphaned
and leak. Seems like we should set a flag to determine if we've already cloned
the cookie, and free the old one if its set. If we wanted to do that on the
cheap, we might be able to get away with checking asoc->stream->[in|out]cnt for
being non-zero as an indicator if we've already cloned the cookie

Neil

2019-05-29 19:10:27

by Neil Horman

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Tue, May 28, 2019 at 07:15:50AM -0400, Neil Horman wrote:
> On Mon, May 27, 2019 at 10:36:00PM -0300, Marcelo Ricardo Leitner wrote:
> > On Mon, May 27, 2019 at 05:48:06PM -0700, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit: 9c7db500 Merge tag 'selinux-pr-20190521' of git://git.kern..
> > > git tree: upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=10388530a00000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=61dd9e15a761691d
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=f7e9153b037eac9b1df8
> > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10e32f8ca00000
> > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=177fa530a00000
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: [email protected]
> > >
> > > 0 to HW filter on device batadv0
> > > executing program
> > > executing program
> > > executing program
> > > BUG: memory leak
> > > unreferenced object 0xffff88810ef68400 (size 1024):
> > > comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.770s)
> > > hex dump (first 32 bytes):
> > > 1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...%
> > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > > backtrace:
> > > [<00000000a02cebbd>] kmemleak_alloc_recursive
> > > include/linux/kmemleak.h:55 [inline]
> > > [<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline]
> > > [<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline]
> > > [<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline]
> > > [<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675
> > > [<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119
> > > [<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline]
> > > [<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20
> > > net/sctp/sm_make_chunk.c:2437
> > > [<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682
> > > [inline]
> > > [<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384
> > > [inline]
> > > [<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194
> > > [inline]
> > > [<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60
> > > net/sctp/sm_sideeffect.c:1165
> >
> > Note that this is on the client side. It was handling the INIT_ACK
> > chunk, from sctp_sf_do_5_1C_ack().
> >
> > I'm not seeing anything else other than sctp_association_free()
> > releasing this memory. This means 2 things:
> > - Every time the cookie is retransmitted, it leaks. As shown by the
> > repetitive leaks here.
> > - The cookie remains allocated throughout the association, which is
> > also not good as that's a 1k that we could have released back to the
> > system right after the handshake.
> >
> > Marcelo
> >
> If we have an INIT chunk bundled with a COOKIE_ECHO chunk in the same packet,
> this might occur. Processing for each chunk (via sctp_cmd_process_init and
> sctp_sf_do_5_1D_ce both call sctp_process_init, which would cause a second write
> to asoc->peer.cookie, leaving the first write (set via kmemdup), to be orphaned
> and leak. Seems like we should set a flag to determine if we've already cloned
> the cookie, and free the old one if its set. If we wanted to do that on the
> cheap, we might be able to get away with checking asoc->stream->[in|out]cnt for
> being non-zero as an indicator if we've already cloned the cookie
>
> Neil
>
>

Completely untested, but can you give this patch a shot?


diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 0767701ef362..a5772d72eb87 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1701,6 +1701,7 @@ struct sctp_association {
__u8 sack_needed:1, /* Do we need to sack the peer? */
sack_generation:1,
zero_window_announced:1;
+ cookie_allocated:1
__u32 sack_cnt;

__u32 adaptation_ind; /* Adaptation Code point. */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 1999237ce481..b6e8fd7081b7 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -213,6 +213,7 @@ static struct sctp_association *sctp_association_init(
*/
asoc->peer.sack_needed = 1;
asoc->peer.sack_generation = 1;
+ asoc->cookie_allocated=0;

/* Assume that the peer will tell us if he recognizes ASCONF
* as part of INIT exchange.
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 92331e1195c1..e966a3cc78bf 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2419,9 +2419,12 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
/* Copy cookie in case we need to resend COOKIE-ECHO. */
cookie = asoc->peer.cookie;
if (cookie) {
+ if (asoc->peer.cookie_allocated)
+ kfree(cookie);
asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
if (!asoc->peer.cookie)
goto clean_up;
+ asoc->peer.cookie_allocated=1;
}

/* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily

2019-05-29 23:39:31

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Wed, May 29, 2019 at 03:07:09PM -0400, Neil Horman wrote:
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2419,9 +2419,12 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> /* Copy cookie in case we need to resend COOKIE-ECHO. */
> cookie = asoc->peer.cookie;
> if (cookie) {
> + if (asoc->peer.cookie_allocated)
> + kfree(cookie);
> asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
> if (!asoc->peer.cookie)
> goto clean_up;
> + asoc->peer.cookie_allocated=1;
> }
>
> /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily

What if we kmemdup directly at sctp_process_param(), as it's done for
others already? Like SCTP_PARAM_RANDOM and SCTP_PARAM_HMAC_ALGO. I
don't see a reason for SCTP_PARAM_STATE_COOKIE to be different
here. This way it would be always allocated, and ready to be kfreed.

We still need to free it after the handshake, btw.

Marcelo

2019-05-30 11:26:32

by Neil Horman

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Wed, May 29, 2019 at 08:37:57PM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, May 29, 2019 at 03:07:09PM -0400, Neil Horman wrote:
> > --- a/net/sctp/sm_make_chunk.c
> > +++ b/net/sctp/sm_make_chunk.c
> > @@ -2419,9 +2419,12 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> > /* Copy cookie in case we need to resend COOKIE-ECHO. */
> > cookie = asoc->peer.cookie;
> > if (cookie) {
> > + if (asoc->peer.cookie_allocated)
> > + kfree(cookie);
> > asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
> > if (!asoc->peer.cookie)
> > goto clean_up;
> > + asoc->peer.cookie_allocated=1;
> > }
> >
> > /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily
>
> What if we kmemdup directly at sctp_process_param(), as it's done for
> others already? Like SCTP_PARAM_RANDOM and SCTP_PARAM_HMAC_ALGO. I
> don't see a reason for SCTP_PARAM_STATE_COOKIE to be different
> here. This way it would be always allocated, and ready to be kfreed.
>
> We still need to free it after the handshake, btw.
>
Yeah, that makes sense, I'll give that a shot.
Neil

> Marcelo
>

2019-05-30 14:23:14

by Neil Horman

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Wed, May 29, 2019 at 08:37:57PM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, May 29, 2019 at 03:07:09PM -0400, Neil Horman wrote:
> > --- a/net/sctp/sm_make_chunk.c
> > +++ b/net/sctp/sm_make_chunk.c
> > @@ -2419,9 +2419,12 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> > /* Copy cookie in case we need to resend COOKIE-ECHO. */
> > cookie = asoc->peer.cookie;
> > if (cookie) {
> > + if (asoc->peer.cookie_allocated)
> > + kfree(cookie);
> > asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
> > if (!asoc->peer.cookie)
> > goto clean_up;
> > + asoc->peer.cookie_allocated=1;
> > }
> >
> > /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily
>
> What if we kmemdup directly at sctp_process_param(), as it's done for
> others already? Like SCTP_PARAM_RANDOM and SCTP_PARAM_HMAC_ALGO. I
> don't see a reason for SCTP_PARAM_STATE_COOKIE to be different
> here. This way it would be always allocated, and ready to be kfreed.
>
> We still need to free it after the handshake, btw.
>
> Marcelo
>

Still untested, but something like this?


diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index d2c7d0d2abc1..718b9917844e 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -393,6 +393,7 @@ void sctp_association_free(struct sctp_association *asoc)
kfree(asoc->peer.peer_random);
kfree(asoc->peer.peer_chunks);
kfree(asoc->peer.peer_hmacs);
+ kfree(asoc->peer.cookie);

/* Release the transport structures. */
list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) {
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 72e74503f9fc..ff365f22a3c1 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2431,14 +2431,6 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
/* Peer Rwnd : Current calculated value of the peer's rwnd. */
asoc->peer.rwnd = asoc->peer.i.a_rwnd;

- /* Copy cookie in case we need to resend COOKIE-ECHO. */
- cookie = asoc->peer.cookie;
- if (cookie) {
- asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
- if (!asoc->peer.cookie)
- goto clean_up;
- }
-
/* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily
* high (for example, implementations MAY use the size of the receiver
* advertised window).
@@ -2607,7 +2599,9 @@ static int sctp_process_param(struct sctp_association *asoc,
case SCTP_PARAM_STATE_COOKIE:
asoc->peer.cookie_len =
ntohs(param.p->length) - sizeof(struct sctp_paramhdr);
- asoc->peer.cookie = param.cookie->body;
+ asoc->peer.cookie = kmemdup(param.cookie->body, asoc->peer.cookie_len, gfp);
+ if (!asoc->peer.cookie)
+ retval = 0;
break;

case SCTP_PARAM_HEARTBEAT_INFO:

2019-05-30 15:18:48

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Thu, May 30, 2019 at 10:20:11AM -0400, Neil Horman wrote:
> On Wed, May 29, 2019 at 08:37:57PM -0300, Marcelo Ricardo Leitner wrote:
> > On Wed, May 29, 2019 at 03:07:09PM -0400, Neil Horman wrote:
> > > --- a/net/sctp/sm_make_chunk.c
> > > +++ b/net/sctp/sm_make_chunk.c
> > > @@ -2419,9 +2419,12 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> > > /* Copy cookie in case we need to resend COOKIE-ECHO. */
> > > cookie = asoc->peer.cookie;
> > > if (cookie) {
> > > + if (asoc->peer.cookie_allocated)
> > > + kfree(cookie);
> > > asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
> > > if (!asoc->peer.cookie)
> > > goto clean_up;
> > > + asoc->peer.cookie_allocated=1;
> > > }
> > >
> > > /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily
> >
> > What if we kmemdup directly at sctp_process_param(), as it's done for
> > others already? Like SCTP_PARAM_RANDOM and SCTP_PARAM_HMAC_ALGO. I
> > don't see a reason for SCTP_PARAM_STATE_COOKIE to be different
> > here. This way it would be always allocated, and ready to be kfreed.
> >
> > We still need to free it after the handshake, btw.
> >
> > Marcelo
> >
>
> Still untested, but something like this?
>

Yes, just..

>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index d2c7d0d2abc1..718b9917844e 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -393,6 +393,7 @@ void sctp_association_free(struct sctp_association *asoc)
> kfree(asoc->peer.peer_random);
> kfree(asoc->peer.peer_chunks);
> kfree(asoc->peer.peer_hmacs);
> + kfree(asoc->peer.cookie);

this chunk is not needed because it is freed right above the first
kfree() in the context here.

>
> /* Release the transport structures. */
> list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) {
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 72e74503f9fc..ff365f22a3c1 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2431,14 +2431,6 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> /* Peer Rwnd : Current calculated value of the peer's rwnd. */
> asoc->peer.rwnd = asoc->peer.i.a_rwnd;
>
> - /* Copy cookie in case we need to resend COOKIE-ECHO. */
> - cookie = asoc->peer.cookie;
> - if (cookie) {
> - asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
> - if (!asoc->peer.cookie)
> - goto clean_up;
> - }
> -
> /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily
> * high (for example, implementations MAY use the size of the receiver
> * advertised window).
> @@ -2607,7 +2599,9 @@ static int sctp_process_param(struct sctp_association *asoc,
> case SCTP_PARAM_STATE_COOKIE:
> asoc->peer.cookie_len =
> ntohs(param.p->length) - sizeof(struct sctp_paramhdr);
> - asoc->peer.cookie = param.cookie->body;
> + asoc->peer.cookie = kmemdup(param.cookie->body, asoc->peer.cookie_len, gfp);
> + if (!asoc->peer.cookie)
> + retval = 0;
> break;
>
> case SCTP_PARAM_HEARTBEAT_INFO:

Plus:

--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -898,6 +898,11 @@ static void sctp_cmd_new_state(struct sctp_cmd_seq *cmds,
asoc->rto_initial;
}

+ if (sctp_state(asoc, ESTABLISHED)) {
+ kfree(asoc->peer.cookie);
+ asoc->peer.cookie = NULL;
+ }
+
if (sctp_state(asoc, ESTABLISHED) ||
sctp_state(asoc, CLOSED) ||
sctp_state(asoc, SHUTDOWN_RECEIVED)) {

Also untested, just sharing the idea.

Marcelo

2019-05-30 19:58:46

by Neil Horman

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Thu, May 30, 2019 at 12:17:05PM -0300, Marcelo Ricardo Leitner wrote:
> On Thu, May 30, 2019 at 10:20:11AM -0400, Neil Horman wrote:
> > On Wed, May 29, 2019 at 08:37:57PM -0300, Marcelo Ricardo Leitner wrote:
> > > On Wed, May 29, 2019 at 03:07:09PM -0400, Neil Horman wrote:
> > > > --- a/net/sctp/sm_make_chunk.c
> > > > +++ b/net/sctp/sm_make_chunk.c
> > > > @@ -2419,9 +2419,12 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> > > > /* Copy cookie in case we need to resend COOKIE-ECHO. */
> > > > cookie = asoc->peer.cookie;
> > > > if (cookie) {
> > > > + if (asoc->peer.cookie_allocated)
> > > > + kfree(cookie);
> > > > asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
> > > > if (!asoc->peer.cookie)
> > > > goto clean_up;
> > > > + asoc->peer.cookie_allocated=1;
> > > > }
> > > >
> > > > /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily
> > >
> > > What if we kmemdup directly at sctp_process_param(), as it's done for
> > > others already? Like SCTP_PARAM_RANDOM and SCTP_PARAM_HMAC_ALGO. I
> > > don't see a reason for SCTP_PARAM_STATE_COOKIE to be different
> > > here. This way it would be always allocated, and ready to be kfreed.
> > >
> > > We still need to free it after the handshake, btw.
> > >
> > > Marcelo
> > >
> >
> > Still untested, but something like this?
> >
>
> Yes, just..
>
> >
> > diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> > index d2c7d0d2abc1..718b9917844e 100644
> > --- a/net/sctp/associola.c
> > +++ b/net/sctp/associola.c
> > @@ -393,6 +393,7 @@ void sctp_association_free(struct sctp_association *asoc)
> > kfree(asoc->peer.peer_random);
> > kfree(asoc->peer.peer_chunks);
> > kfree(asoc->peer.peer_hmacs);
> > + kfree(asoc->peer.cookie);
>
> this chunk is not needed because it is freed right above the first
> kfree() in the context here.
>
Ah, thanks, missed that.

> >
> > /* Release the transport structures. */
> > list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) {
> > diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> > index 72e74503f9fc..ff365f22a3c1 100644
> > --- a/net/sctp/sm_make_chunk.c
> > +++ b/net/sctp/sm_make_chunk.c
> > @@ -2431,14 +2431,6 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> > /* Peer Rwnd : Current calculated value of the peer's rwnd. */
> > asoc->peer.rwnd = asoc->peer.i.a_rwnd;
> >
> > - /* Copy cookie in case we need to resend COOKIE-ECHO. */
> > - cookie = asoc->peer.cookie;
> > - if (cookie) {
> > - asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp);
> > - if (!asoc->peer.cookie)
> > - goto clean_up;
> > - }
> > -
> > /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily
> > * high (for example, implementations MAY use the size of the receiver
> > * advertised window).
> > @@ -2607,7 +2599,9 @@ static int sctp_process_param(struct sctp_association *asoc,
> > case SCTP_PARAM_STATE_COOKIE:
> > asoc->peer.cookie_len =
> > ntohs(param.p->length) - sizeof(struct sctp_paramhdr);
> > - asoc->peer.cookie = param.cookie->body;
> > + asoc->peer.cookie = kmemdup(param.cookie->body, asoc->peer.cookie_len, gfp);
> > + if (!asoc->peer.cookie)
> > + retval = 0;
> > break;
> >
> > case SCTP_PARAM_HEARTBEAT_INFO:
>
> Plus:
>
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -898,6 +898,11 @@ static void sctp_cmd_new_state(struct sctp_cmd_seq *cmds,
> asoc->rto_initial;
> }
>
> + if (sctp_state(asoc, ESTABLISHED)) {
> + kfree(asoc->peer.cookie);
> + asoc->peer.cookie = NULL;
> + }
> +
Not sure I follow why this is needed. It doesn't hurt anything of course, but
if we're freeing in sctp_association_free, we don't need to duplicate the
operation here, do we?
> if (sctp_state(asoc, ESTABLISHED) ||
> sctp_state(asoc, CLOSED) ||
> sctp_state(asoc, SHUTDOWN_RECEIVED)) {
>
> Also untested, just sharing the idea.
>
> Marcelo
>

2019-05-31 12:45:24

by Marcelo Ricardo Leitner

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Thu, May 30, 2019 at 03:56:34PM -0400, Neil Horman wrote:
> On Thu, May 30, 2019 at 12:17:05PM -0300, Marcelo Ricardo Leitner wrote:
...
> > --- a/net/sctp/sm_sideeffect.c
> > +++ b/net/sctp/sm_sideeffect.c
> > @@ -898,6 +898,11 @@ static void sctp_cmd_new_state(struct sctp_cmd_seq *cmds,
> > asoc->rto_initial;
> > }
> >
> > + if (sctp_state(asoc, ESTABLISHED)) {
> > + kfree(asoc->peer.cookie);
> > + asoc->peer.cookie = NULL;
> > + }
> > +
> Not sure I follow why this is needed. It doesn't hurt anything of course, but
> if we're freeing in sctp_association_free, we don't need to duplicate the
> operation here, do we?

This one would be to avoid storing the cookie throughout the entire
association lifetime, as the cookie is only needed during the
handshake.
While the free in sctp_association_free will handle the freeing in
case the association never enters established state.

> > if (sctp_state(asoc, ESTABLISHED) ||
> > sctp_state(asoc, CLOSED) ||
> > sctp_state(asoc, SHUTDOWN_RECEIVED)) {
> >
> > Also untested, just sharing the idea.
> >
> > Marcelo
> >

2019-05-31 16:45:38

by Neil Horman

[permalink] [raw]
Subject: Re: memory leak in sctp_process_init

On Fri, May 31, 2019 at 09:42:42AM -0300, Marcelo Ricardo Leitner wrote:
> On Thu, May 30, 2019 at 03:56:34PM -0400, Neil Horman wrote:
> > On Thu, May 30, 2019 at 12:17:05PM -0300, Marcelo Ricardo Leitner wrote:
> ...
> > > --- a/net/sctp/sm_sideeffect.c
> > > +++ b/net/sctp/sm_sideeffect.c
> > > @@ -898,6 +898,11 @@ static void sctp_cmd_new_state(struct sctp_cmd_seq *cmds,
> > > asoc->rto_initial;
> > > }
> > >
> > > + if (sctp_state(asoc, ESTABLISHED)) {
> > > + kfree(asoc->peer.cookie);
> > > + asoc->peer.cookie = NULL;
> > > + }
> > > +
> > Not sure I follow why this is needed. It doesn't hurt anything of course, but
> > if we're freeing in sctp_association_free, we don't need to duplicate the
> > operation here, do we?
>
> This one would be to avoid storing the cookie throughout the entire
> association lifetime, as the cookie is only needed during the
> handshake.
> While the free in sctp_association_free will handle the freeing in
> case the association never enters established state.
>

Ok, I see we do that with the peer_random and other allocated values as well
when they are no longer needed, but ew, I hate freeing in multiple places like
that. I'll fix this up on monday, but I wonder if we can't consolidate that
somehow

Neil

> > > if (sctp_state(asoc, ESTABLISHED) ||
> > > sctp_state(asoc, CLOSED) ||
> > > sctp_state(asoc, SHUTDOWN_RECEIVED)) {
> > >
> > > Also untested, just sharing the idea.
> > >
> > > Marcelo
> > >
>