net/sctp: use-after-free in __sctp_connect

net/sctp: use-after-free in __sctp_connect

[Dmitry Vyukov]

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=15267
[<      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=15267
[<      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=27408 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).

net/sctp: use-after-free in __sctp_connect

[Dmitry Vyukov]

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).

RE: net/sctp: use-after-free in __sctp_connect

[YUAN Jia]

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

-----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

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=15267
[<      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=15267
[<      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=27408 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).
--
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

RE: net/sctp: use-after-free in __sctp_connect

[YUAN Jia]

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=

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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=15267
> [<      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...

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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...

Re: net/sctp: use-after-free in __sctp_connect

[Vlad Yasevich]

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=15267
>> [<      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

Re: net/sctp: use-after-free in __sctp_connect

[Vlad Yasevich]

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

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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=15267
> >> [<      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=AF_INET6, sin6_port=htons(13280),
inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=1882116169,
sin6_scope_id=3305060172}, 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=AF_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=AF_INET6, sin6_port=htons(13276), inet_pton(AF_INET6,
"::1", &sin6_addr), sin6_flowinfo=3512421652, sin6_scope_id=4260889053},
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

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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=15267
> > >> [<      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=AF_INET6, sin6_port=htons(13280),
> inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=1882116169,
> sin6_scope_id=3305060172}, 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=AF_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=AF_INET6, sin6_port=htons(13276), inet_pton(AF_INET6,
> "::1", &sin6_addr), sin6_flowinfo=3512421652, sin6_scope_id=4260889053},
> 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
> 

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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
> 

net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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).

net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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).

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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.

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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.

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

[-- 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;
}

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

[-- 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;
}

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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.

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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.

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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:

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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:

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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:

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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:

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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;

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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;

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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;

Re: net/sctp: use-after-free in __sctp_connect

[Andrey Konovalov]

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;

Re: net/sctp: use-after-free in __sctp_connect

[Neil Horman]

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

Re: net/sctp: use-after-free in __sctp_connect

[Neil Horman]

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

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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

Re: net/sctp: use-after-free in __sctp_connect

[Marcelo Ricardo Leitner]

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