linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* net/sctp: use-after-free in __sctp_connect
@ 2016-01-13  9:52 Dmitry Vyukov
  2016-01-14  1:37 ` YUAN Jia
  2016-01-15 19:01 ` Marcelo Ricardo Leitner
  0 siblings, 2 replies; 17+ messages in thread
From: Dmitry Vyukov @ 2016-01-13  9:52 UTC (permalink / raw)
  To: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, Eric Dumazet, Marcelo Ricardo Leitner
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin

Hello,

The following program causes use-after-free in __sctp_connect:

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <unistd.h>
#include <sys/syscall.h>
#include <string.h>
#include <stdint.h>
#include <pthread.h>

long r[13];

void *thr(void *arg)
{
        switch ((long)arg) {
        case 0:
                r[0] = syscall(SYS_mmap, 0x20000000ul, 0x20000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
                break;
        case 1:
                r[1] = syscall(SYS_socket, 0xaul, 0x1ul, 0x84ul, 0, 0, 0);
                break;
        case 2:
                memcpy((void*)0x2000b000,
"\x0a\x00\x33\xe0\x49\xd0\x2e\x70\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x4c\x37\xff\xc4",
28);
                r[3] = syscall(SYS_bind, r[1], 0x2000b000ul, 0x1cul, 0, 0, 0);
                break;
        case 3:
                r[4] = syscall(SYS_dup2, r[1], r[1], 0, 0, 0, 0);
                break;
        case 4:
                *(uint64_t*)0x200177ed = (uint64_t)0x0;
                *(uint64_t*)0x200177f5 = (uint64_t)0x2710;
                r[7] = syscall(SYS_setsockopt, r[4], 0x1ul, 0x15ul,
0x200177edul, 0x10ul, 0);
                break;
        case 5:
                *(uint16_t*)0x2000b008 = (uint16_t)0x2;
                *(uint16_t*)0x2000b00a = (uint16_t)0xbab;
                *(uint32_t*)0x2000b00c = (uint32_t)0xffffffff;
                r[11] = syscall(SYS_setsockopt, r[4], 0x84ul, 0x6eul,
0x2000b000ul, 0x1cul, 0);
                break;
        case 6:
                r[12] = syscall(SYS_shutdown, r[4], 0x1ul, 0, 0, 0, 0);
                break;
        }
        return 0;
}

int main()
{
        long i;
        pthread_t th[7];

        memset(r, -1, sizeof(r));
        for (i = 0; i < 7; i++) {
                pthread_create(&th[i], 0, thr, (void*)i);
                usleep(10000);
        }
        for (i = 0; i < 7; i++) {
                pthread_create(&th[i], 0, thr, (void*)i);
                if (i%2==0)
                        usleep(10000);
        }
        usleep(100000);
        return 0;
}

==================================================================
BUG: KASAN: use-after-free in __sctp_connect+0xb23/0xb90 at addr
ffff8800605febb8
Read of size 4 by task syz-executor/15263

INFO: Allocated in sctp_association_new+0x6f/0x1da0 age=0 cpu=3 pid=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).

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: net/sctp: use-after-free in __sctp_connect
  2016-01-13  9:52 net/sctp: use-after-free in __sctp_connect Dmitry Vyukov
@ 2016-01-14  1:37 ` YUAN Jia
  2016-01-14  1:45   ` Marcelo Ricardo Leitner
  2016-01-15 19:01 ` Marcelo Ricardo Leitner
  1 sibling, 1 reply; 17+ messages in thread
From: YUAN Jia @ 2016-01-14  1:37 UTC (permalink / raw)
  To: 'Dmitry Vyukov',
	Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, Eric Dumazet, Marcelo Ricardo Leitner
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-01-14  1:37 ` YUAN Jia
@ 2016-01-14  1:45   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-01-14  1:45 UTC (permalink / raw)
  To: YUAN Jia, 'Dmitry Vyukov',
	Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, Eric Dumazet
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin

Em 13-01-2016 23:37, YUAN Jia escreveu:
 > Hi Dmitry,
 >
 > I've tested your program, but found nothing happens. So, I suspect it 
may cause kernel crash only on some special kernel version?
 > I was just using kernel of 4.2.3. What about you?
 >
 > Richard

Please don't top-post.

Note that it doesn't necessarily crashes the system. syzkaller is using 
kasan to detect such type of invalid accesses, which in some conditions 
may lead to crashes, weird behavior or just nothing at all. It all 
depends on how much the memory was already changed after it was freed.

 > -----Original Message-----
 > From: linux-sctp-owner@vger.kernel.org 
[mailto:linux-sctp-owner@vger.kernel.org] On Behalf Of Dmitry Vyukov
 > Sent: 2016年1月13日 17:53
 > To: Vlad Yasevich; Neil Horman; David S. Miller; 
linux-sctp@vger.kernel.org; netdev; LKML; Eric Dumazet; Marcelo Ricardo 
Leitner
 > Cc: syzkaller; Kostya Serebryany; Alexander Potapenko; Sasha Levin
 > Subject: net/sctp: use-after-free in __sctp_connect
 >

...

 >
 > On commit 03891f9c853d5c4473224478a1e03ea00d70ff8d (Jan 11).

This is the git commit/(version) he was using --^

Marcelo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-01-13  9:52 net/sctp: use-after-free in __sctp_connect Dmitry Vyukov
  2016-01-14  1:37 ` YUAN Jia
@ 2016-01-15 19:01 ` Marcelo Ricardo Leitner
  2016-01-19 14:38   ` Vlad Yasevich
  1 sibling, 1 reply; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-01-15 19:01 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, Eric Dumazet, syzkaller, Kostya Serebryany,
	Alexander Potapenko, Sasha Levin

On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote:
> Hello,
> 
> The following program causes use-after-free in __sctp_connect:
> 
...
> INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid=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...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-01-15 19:01 ` Marcelo Ricardo Leitner
@ 2016-01-19 14:38   ` Vlad Yasevich
  2016-01-21 17:18     ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 17+ messages in thread
From: Vlad Yasevich @ 2016-01-19 14:38 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, Dmitry Vyukov
  Cc: Neil Horman, David S. Miller, linux-sctp, netdev, LKML,
	Eric Dumazet, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Sasha Levin

On 01/15/2016 02:01 PM, Marcelo Ricardo Leitner wrote:
> On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> The following program causes use-after-free in __sctp_connect:
>>
> ...
>> INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid=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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-01-19 14:38   ` Vlad Yasevich
@ 2016-01-21 17:18     ` Marcelo Ricardo Leitner
  2016-01-21 17:37       ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-01-21 17:18 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: Dmitry Vyukov, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, Eric Dumazet, syzkaller, Kostya Serebryany,
	Alexander Potapenko, Sasha Levin

On Tue, Jan 19, 2016 at 09:38:54AM -0500, Vlad Yasevich wrote:
> On 01/15/2016 02:01 PM, Marcelo Ricardo Leitner wrote:
> > On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote:
> >> Hello,
> >>
> >> The following program causes use-after-free in __sctp_connect:
> >>
> > ...
> >> INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid=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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-01-21 17:18     ` Marcelo Ricardo Leitner
@ 2016-01-21 17:37       ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-01-21 17:37 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: Dmitry Vyukov, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, Eric Dumazet, syzkaller, Kostya Serebryany,
	Alexander Potapenko, Sasha Levin

On Thu, Jan 21, 2016 at 03:18:18PM -0200, Marcelo Ricardo Leitner wrote:
> On Tue, Jan 19, 2016 at 09:38:54AM -0500, Vlad Yasevich wrote:
> > On 01/15/2016 02:01 PM, Marcelo Ricardo Leitner wrote:
> > > On Wed, Jan 13, 2016 at 10:52:31AM +0100, Dmitry Vyukov wrote:
> > >> Hello,
> > >>
> > >> The following program causes use-after-free in __sctp_connect:
> > >>
> > > ...
> > >> INFO: Freed in sctp_association_put+0x150/0x250 age=0 cpu=3 pid=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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-11-04 12:59             ` Neil Horman
@ 2016-11-04 13:03               ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-11-04 13:03 UTC (permalink / raw)
  To: Neil Horman
  Cc: Andrey Konovalov, Vlad Yasevich, David S. Miller, linux-sctp,
	netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

On Fri, Nov 04, 2016 at 08:59:58AM -0400, Neil Horman wrote:
> On Thu, Nov 03, 2016 at 04:35:33PM -0200, Marcelo Ricardo Leitner wrote:
> > On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote:
> > > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner
> > > <marcelo.leitner@gmail.com> wrote:
> > > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote:
> > > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> > > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
> > > >> > <marcelo.leitner@gmail.com> wrote:
> > > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
> > > >> >>> Hi,
> > > >> >>>
> > > >> >>> I've got the following error report while running the syzkaller fuzzer:
> > > >> >>>
> > > >> >>> ==================================================================
> > > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
> > > >> >>> ffff88006b1dc610
> > > >> >>
> > > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
> > > >> >> So far I couldn't identify the reason.
> > > >> >> "Good" to know it's still there, thanks for reporting it.
> > > >>
> > > >> Hi Marcelo,
> > > >>
> > > >
> > > > Hi
> > > >
> > > >> So I've looked at the code.
> > > >> As far as I understand, the problem is a race condition between
> > > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
> > > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop
> > > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
> > > >> with sctp_association_put() and returns err = 0.
> > > >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
> > > >> from the freed asoc.
> > > >
> > > > Suddenly this seems familiar. Your description makes sense, thanks for
> > > > looking deeper into this, Andrey.
> > > >
> > > > This fix should do it, can you please try it? I'll post it properly
> > > > if it works.
> > > 
> > > Yes, it fixes the issue.
> > > 
> > > Tested-by: Andrey Konovalov <andreyknvl@google.com>
> > > 
> > > Thanks for the fix!
> > 
> > Ahm this other fix looks better: do the read before calling
> > sctp_wait_for_connect() as that id won't change in this call and the
> > application shouldn't trust this number if an error is returned, so
> > there should be no issues by returning it in such situation.
> > 
> > Can you please confirm this one also works? Thanks!
> > 
> > ---8<---
> > 
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 6cdc61c21438..be1d9bb98230 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk,
> >  
> >  	timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
> >  
> > -	err = sctp_wait_for_connect(asoc, &timeo);
> > -	if ((err == 0 || err == -EINPROGRESS) && assoc_id)
> > +	if (assoc_id)
> >  		*assoc_id = asoc->assoc_id;
> > +	err = sctp_wait_for_connect(asoc, &timeo);
> > +	/* Note: the asoc may be freed after the return of
> > +	 * sctp_wait_for_connect.
> > +	 */
> >  
> >  	/* Don't free association on exit. */
> >  	asoc = NULL;
> > 
> Agreed, this version looks better.  Please repost it with a proper changelog and
> I'll ack it.
> Neil

It already is. :-)
Please look for
Subject: [PATCH net] sctp: assign assoc_id earlier in __sctp_connect            

Thanks,
Marcelo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-11-03 18:35           ` Marcelo Ricardo Leitner
  2016-11-03 18:45             ` Andrey Konovalov
@ 2016-11-04 12:59             ` Neil Horman
  2016-11-04 13:03               ` Marcelo Ricardo Leitner
  1 sibling, 1 reply; 17+ messages in thread
From: Neil Horman @ 2016-11-04 12:59 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Andrey Konovalov, Vlad Yasevich, David S. Miller, linux-sctp,
	netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

On Thu, Nov 03, 2016 at 04:35:33PM -0200, Marcelo Ricardo Leitner wrote:
> On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote:
> > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote:
> > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
> > >> > <marcelo.leitner@gmail.com> wrote:
> > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
> > >> >>> Hi,
> > >> >>>
> > >> >>> I've got the following error report while running the syzkaller fuzzer:
> > >> >>>
> > >> >>> ==================================================================
> > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
> > >> >>> ffff88006b1dc610
> > >> >>
> > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
> > >> >> So far I couldn't identify the reason.
> > >> >> "Good" to know it's still there, thanks for reporting it.
> > >>
> > >> Hi Marcelo,
> > >>
> > >
> > > Hi
> > >
> > >> So I've looked at the code.
> > >> As far as I understand, the problem is a race condition between
> > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
> > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop
> > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
> > >> with sctp_association_put() and returns err = 0.
> > >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
> > >> from the freed asoc.
> > >
> > > Suddenly this seems familiar. Your description makes sense, thanks for
> > > looking deeper into this, Andrey.
> > >
> > > This fix should do it, can you please try it? I'll post it properly
> > > if it works.
> > 
> > Yes, it fixes the issue.
> > 
> > Tested-by: Andrey Konovalov <andreyknvl@google.com>
> > 
> > Thanks for the fix!
> 
> Ahm this other fix looks better: do the read before calling
> sctp_wait_for_connect() as that id won't change in this call and the
> application shouldn't trust this number if an error is returned, so
> there should be no issues by returning it in such situation.
> 
> Can you please confirm this one also works? Thanks!
> 
> ---8<---
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 6cdc61c21438..be1d9bb98230 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk,
>  
>  	timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
>  
> -	err = sctp_wait_for_connect(asoc, &timeo);
> -	if ((err == 0 || err == -EINPROGRESS) && assoc_id)
> +	if (assoc_id)
>  		*assoc_id = asoc->assoc_id;
> +	err = sctp_wait_for_connect(asoc, &timeo);
> +	/* Note: the asoc may be freed after the return of
> +	 * sctp_wait_for_connect.
> +	 */
>  
>  	/* Don't free association on exit. */
>  	asoc = NULL;
> 
Agreed, this version looks better.  Please repost it with a proper changelog and
I'll ack it.
Neil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-11-03 18:35           ` Marcelo Ricardo Leitner
@ 2016-11-03 18:45             ` Andrey Konovalov
  2016-11-04 12:59             ` Neil Horman
  1 sibling, 0 replies; 17+ messages in thread
From: Andrey Konovalov @ 2016-11-03 18:45 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

On Thu, Nov 3, 2016 at 7:35 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote:
>> On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote:
>> >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
>> >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
>> >> > <marcelo.leitner@gmail.com> wrote:
>> >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
>> >> >>> Hi,
>> >> >>>
>> >> >>> I've got the following error report while running the syzkaller fuzzer:
>> >> >>>
>> >> >>> ==================================================================
>> >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
>> >> >>> ffff88006b1dc610
>> >> >>
>> >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
>> >> >> So far I couldn't identify the reason.
>> >> >> "Good" to know it's still there, thanks for reporting it.
>> >>
>> >> Hi Marcelo,
>> >>
>> >
>> > Hi
>> >
>> >> So I've looked at the code.
>> >> As far as I understand, the problem is a race condition between
>> >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
>> >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop
>> >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
>> >> with sctp_association_put() and returns err = 0.
>> >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
>> >> from the freed asoc.
>> >
>> > Suddenly this seems familiar. Your description makes sense, thanks for
>> > looking deeper into this, Andrey.
>> >
>> > This fix should do it, can you please try it? I'll post it properly
>> > if it works.
>>
>> Yes, it fixes the issue.
>>
>> Tested-by: Andrey Konovalov <andreyknvl@google.com>
>>
>> Thanks for the fix!
>
> Ahm this other fix looks better: do the read before calling
> sctp_wait_for_connect() as that id won't change in this call and the
> application shouldn't trust this number if an error is returned, so
> there should be no issues by returning it in such situation.
>
> Can you please confirm this one also works? Thanks!

Sure!

This one also works.

Tested-by: Andrey Konovalov <andreyknvl@google.com>

>
> ---8<---
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 6cdc61c21438..be1d9bb98230 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk,
>
>         timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
>
> -       err = sctp_wait_for_connect(asoc, &timeo);
> -       if ((err == 0 || err == -EINPROGRESS) && assoc_id)
> +       if (assoc_id)
>                 *assoc_id = asoc->assoc_id;
> +       err = sctp_wait_for_connect(asoc, &timeo);
> +       /* Note: the asoc may be freed after the return of
> +        * sctp_wait_for_connect.
> +        */
>
>         /* Don't free association on exit. */
>         asoc = NULL;

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-11-03 18:02         ` Andrey Konovalov
@ 2016-11-03 18:35           ` Marcelo Ricardo Leitner
  2016-11-03 18:45             ` Andrey Konovalov
  2016-11-04 12:59             ` Neil Horman
  0 siblings, 2 replies; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-11-03 18:35 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote:
> On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote:
> >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
> >> > <marcelo.leitner@gmail.com> wrote:
> >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
> >> >>> Hi,
> >> >>>
> >> >>> I've got the following error report while running the syzkaller fuzzer:
> >> >>>
> >> >>> ==================================================================
> >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
> >> >>> ffff88006b1dc610
> >> >>
> >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
> >> >> So far I couldn't identify the reason.
> >> >> "Good" to know it's still there, thanks for reporting it.
> >>
> >> Hi Marcelo,
> >>
> >
> > Hi
> >
> >> So I've looked at the code.
> >> As far as I understand, the problem is a race condition between
> >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
> >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop
> >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
> >> with sctp_association_put() and returns err = 0.
> >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
> >> from the freed asoc.
> >
> > Suddenly this seems familiar. Your description makes sense, thanks for
> > looking deeper into this, Andrey.
> >
> > This fix should do it, can you please try it? I'll post it properly
> > if it works.
> 
> Yes, it fixes the issue.
> 
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> 
> Thanks for the fix!

Ahm this other fix looks better: do the read before calling
sctp_wait_for_connect() as that id won't change in this call and the
application shouldn't trust this number if an error is returned, so
there should be no issues by returning it in such situation.

Can you please confirm this one also works? Thanks!

---8<---

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 6cdc61c21438..be1d9bb98230 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk,
 
 	timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
 
-	err = sctp_wait_for_connect(asoc, &timeo);
-	if ((err == 0 || err == -EINPROGRESS) && assoc_id)
+	if (assoc_id)
 		*assoc_id = asoc->assoc_id;
+	err = sctp_wait_for_connect(asoc, &timeo);
+	/* Note: the asoc may be freed after the return of
+	 * sctp_wait_for_connect.
+	 */
 
 	/* Don't free association on exit. */
 	asoc = NULL;

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-11-03 17:52       ` Marcelo Ricardo Leitner
@ 2016-11-03 18:02         ` Andrey Konovalov
  2016-11-03 18:35           ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Konovalov @ 2016-11-03 18:02 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote:
>> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
>> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
>> > <marcelo.leitner@gmail.com> wrote:
>> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
>> >>> Hi,
>> >>>
>> >>> I've got the following error report while running the syzkaller fuzzer:
>> >>>
>> >>> ==================================================================
>> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
>> >>> ffff88006b1dc610
>> >>
>> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
>> >> So far I couldn't identify the reason.
>> >> "Good" to know it's still there, thanks for reporting it.
>>
>> Hi Marcelo,
>>
>
> Hi
>
>> So I've looked at the code.
>> As far as I understand, the problem is a race condition between
>> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
>> setsockopt() calls sctp_wait_for_connect(), which exits the for loop
>> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
>> with sctp_association_put() and returns err = 0.
>> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
>> from the freed asoc.
>
> Suddenly this seems familiar. Your description makes sense, thanks for
> looking deeper into this, Andrey.
>
> This fix should do it, can you please try it? I'll post it properly
> if it works.

Yes, it fixes the issue.

Tested-by: Andrey Konovalov <andreyknvl@google.com>

Thanks for the fix!

>
> wait_for_connect is only used in two places, we can move the ref to a
> broader scope and cover that read too, instead of holding another ref.
>
> sendmsg path won't read anything from the asoc after waiting, so this
> should be enough for it too.
>
> ---8<---
>
> commit 7f7ba9b4fb834a61ab097dfd7c1f267e6a6d70a8
> Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Date:   Thu Nov 3 15:47:45 2016 -0200
>
>     sctp: hold the asoc longer when associating
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9fbb6feb8c27..aac271571930 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1214,9 +1214,11 @@ static int __sctp_connect(struct sock *sk,
>
>         timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
>
> +       sctp_association_hold(asoc);
>         err = sctp_wait_for_connect(asoc, &timeo);
>         if ((err == 0 || err == -EINPROGRESS) && assoc_id)
>                 *assoc_id = asoc->assoc_id;
> +       sctp_association_put(asoc);
>
>         /* Don't free association on exit. */
>         asoc = NULL;
> @@ -1985,7 +1987,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
>
>         if (unlikely(wait_connect)) {
>                 timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT);
> +               sctp_association_hold(asoc);
>                 sctp_wait_for_connect(asoc, &timeo);
> +               sctp_association_put(asoc);
>         }
>
>         /* If we are already past ASSOCIATE, the lower
> @@ -7501,6 +7505,7 @@ static int sctp_writeable(struct sock *sk)
>
>  /* Wait for an association to go into ESTABLISHED state. If timeout is 0,
>   * returns immediately with EINPROGRESS.
> + * Note: caller must hold a ref on asoc before calling this function.
>   */
>  static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p)
>  {
> @@ -7511,9 +7516,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p)
>
>         pr_debug("%s: asoc:%p, timeo:%ld\n", __func__, asoc, *timeo_p);
>
> -       /* Increment the association's refcnt.  */
> -       sctp_association_hold(asoc);
> -
>         for (;;) {
>                 prepare_to_wait_exclusive(&asoc->wait, &wait,
>                                           TASK_INTERRUPTIBLE);
> @@ -7543,9 +7545,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p)
>  out:
>         finish_wait(&asoc->wait, &wait);
>
> -       /* Release the association's refcnt.  */
> -       sctp_association_put(asoc);
> -
>         return err;
>
>  do_error:

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-11-03 17:11     ` Andrey Konovalov
@ 2016-11-03 17:52       ` Marcelo Ricardo Leitner
  2016-11-03 18:02         ` Andrey Konovalov
  0 siblings, 1 reply; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-11-03 17:52 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote:
> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
> >>> Hi,
> >>>
> >>> I've got the following error report while running the syzkaller fuzzer:
> >>>
> >>> ==================================================================
> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
> >>> ffff88006b1dc610
> >>
> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
> >> So far I couldn't identify the reason.
> >> "Good" to know it's still there, thanks for reporting it.
> 
> Hi Marcelo,
> 

Hi

> So I've looked at the code.
> As far as I understand, the problem is a race condition between
> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
> setsockopt() calls sctp_wait_for_connect(), which exits the for loop
> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
> with sctp_association_put() and returns err = 0.
> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
> from the freed asoc.

Suddenly this seems familiar. Your description makes sense, thanks for
looking deeper into this, Andrey.

This fix should do it, can you please try it? I'll post it properly
if it works.

wait_for_connect is only used in two places, we can move the ref to a
broader scope and cover that read too, instead of holding another ref.

sendmsg path won't read anything from the asoc after waiting, so this
should be enough for it too.

---8<---

commit 7f7ba9b4fb834a61ab097dfd7c1f267e6a6d70a8
Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date:   Thu Nov 3 15:47:45 2016 -0200

    sctp: hold the asoc longer when associating

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9fbb6feb8c27..aac271571930 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1214,9 +1214,11 @@ static int __sctp_connect(struct sock *sk,
 
 	timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK);
 
+	sctp_association_hold(asoc);
 	err = sctp_wait_for_connect(asoc, &timeo);
 	if ((err == 0 || err == -EINPROGRESS) && assoc_id)
 		*assoc_id = asoc->assoc_id;
+	sctp_association_put(asoc);
 
 	/* Don't free association on exit. */
 	asoc = NULL;
@@ -1985,7 +1987,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
 
 	if (unlikely(wait_connect)) {
 		timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT);
+		sctp_association_hold(asoc);
 		sctp_wait_for_connect(asoc, &timeo);
+		sctp_association_put(asoc);
 	}
 
 	/* If we are already past ASSOCIATE, the lower
@@ -7501,6 +7505,7 @@ static int sctp_writeable(struct sock *sk)
 
 /* Wait for an association to go into ESTABLISHED state. If timeout is 0,
  * returns immediately with EINPROGRESS.
+ * Note: caller must hold a ref on asoc before calling this function.
  */
 static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p)
 {
@@ -7511,9 +7516,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p)
 
 	pr_debug("%s: asoc:%p, timeo:%ld\n", __func__, asoc, *timeo_p);
 
-	/* Increment the association's refcnt.  */
-	sctp_association_hold(asoc);
-
 	for (;;) {
 		prepare_to_wait_exclusive(&asoc->wait, &wait,
 					  TASK_INTERRUPTIBLE);
@@ -7543,9 +7545,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p)
 out:
 	finish_wait(&asoc->wait, &wait);
 
-	/* Release the association's refcnt.  */
-	sctp_association_put(asoc);
-
 	return err;
 
 do_error:

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-11-02 22:42   ` Andrey Konovalov
@ 2016-11-03 17:11     ` Andrey Konovalov
  2016-11-03 17:52       ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Konovalov @ 2016-11-03 17:11 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
>> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
>>> Hi,
>>>
>>> I've got the following error report while running the syzkaller fuzzer:
>>>
>>> ==================================================================
>>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
>>> ffff88006b1dc610
>>
>> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
>> So far I couldn't identify the reason.
>> "Good" to know it's still there, thanks for reporting it.

Hi Marcelo,

So I've looked at the code.
As far as I understand, the problem is a race condition between
setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
setsockopt() calls sctp_wait_for_connect(), which exits the for loop
on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
with sctp_association_put() and returns err = 0.
Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
from the freed asoc.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-10-19 16:57 ` Marcelo Ricardo Leitner
@ 2016-11-02 22:42   ` Andrey Konovalov
  2016-11-03 17:11     ` Andrey Konovalov
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Konovalov @ 2016-11-02 22:42 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]

On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
>> Hi,
>>
>> I've got the following error report while running the syzkaller fuzzer:
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
>> ffff88006b1dc610
>
> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
> So far I couldn't identify the reason.
> "Good" to know it's still there, thanks for reporting it.

Hi Marcelo,

I've attached a reproducer that might help to figure out the reason.

It triggers the UAF for me in ~10 seconds of running as:
$ gcc -lpthread sctp-connect-uaf-poc.c
$ while true; do ./a.out; done

You need to have KASAN enabled.

>

[-- Attachment #2: sctp-connect-uaf-poc.c --]
[-- Type: application/octet-stream, Size: 3735 bytes --]

// autogenerated by syzkaller (http://github.com/google/syzkaller)

#ifndef __NR_socket
#define __NR_socket 41
#endif
#ifndef __NR_setsockopt
#define __NR_setsockopt 54
#endif
#ifndef __NR_mmap
#define __NR_mmap 9
#endif
#ifndef __NR_shutdown
#define __NR_shutdown 48
#endif

#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>

#include <errno.h>
#include <error.h>
#include <fcntl.h>
#include <pthread.h>
#include <setjmp.h>
#include <signal.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

__thread int skip_segv;
__thread jmp_buf segv_env;

static void segv_handler(int sig, siginfo_t* info, void* uctx)
{
  if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED))
    _longjmp(segv_env, 1);
  exit(sig);
}

static void install_segv_handler()
{
  struct sigaction sa;
  memset(&sa, 0, sizeof(sa));
  sa.sa_sigaction = segv_handler;
  sa.sa_flags = SA_NODEFER | SA_SIGINFO;
  sigaction(SIGSEGV, &sa, NULL);
  sigaction(SIGBUS, &sa, NULL);
}

#define NONFAILING(...)                                                \
  {                                                                    \
    __atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
    if (_setjmp(segv_env) == 0) {                                      \
      __VA_ARGS__;                                                     \
    }                                                                  \
    __atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
  }

static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1,
                                 uintptr_t a2, uintptr_t a3,
                                 uintptr_t a4, uintptr_t a5,
                                 uintptr_t a6, uintptr_t a7,
                                 uintptr_t a8)
{
  switch (nr) {
  default:
    return syscall(nr, a0, a1, a2, a3, a4, a5);
  }
}

long r[14];
void* thr(void* arg)
{
  switch ((long)arg) {
  case 0:
    r[0] =
        execute_syscall(__NR_mmap, 0x20000000ul, 0x332000ul, 0x3ul,
                        0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
    break;
  case 1:
    r[1] = execute_syscall(__NR_socket, 0x2ul, 0x1ul, 0x0ul, 0, 0, 0, 0,
                           0, 0);
    break;
  case 2:
    r[2] = execute_syscall(__NR_socket, 0xaul, 0x1ul, 0x84ul, 0, 0, 0,
                           0, 0, 0);
    break;
  case 3:
    r[3] = execute_syscall(__NR_shutdown, r[2], 0x0ul, 0, 0, 0, 0, 0, 0,
                           0);
    break;
  case 4:
    NONFAILING(*(uint16_t*)0x20008fe4 = (uint16_t)0xa);
    NONFAILING(*(uint16_t*)0x20008fe6 = (uint16_t)0x4242);
    NONFAILING(*(uint32_t*)0x20008fe8 = (uint32_t)0x1ddb);
    NONFAILING(*(uint32_t*)0x20008fec = (uint32_t)0x5);
    NONFAILING(*(uint32_t*)0x20008ff0 = (uint32_t)0xffff);
    NONFAILING(*(uint32_t*)0x20008ff4 = (uint32_t)0x7);
    NONFAILING(*(uint32_t*)0x20008ff8 = (uint32_t)0x0);
    NONFAILING(*(uint32_t*)0x20008ffc = (uint32_t)0xffffffffffffff23);
    r[12] = execute_syscall(__NR_setsockopt, r[2], 0x84ul, 0x6eul,
                            0x20008fe4ul, 0x1cul, 0, 0, 0, 0);
    break;
  case 5:
    r[13] = execute_syscall(__NR_shutdown, r[2], 0x1ul, 0, 0, 0, 0, 0,
                            0, 0);
    break;
  }
  return 0;
}

int main()
{
  long i;
  pthread_t th[12];

  install_segv_handler();
  memset(r, -1, sizeof(r));
  srand(getpid());
  for (i = 0; i < 6; i++) {
    pthread_create(&th[i], 0, thr, (void*)i);
    usleep(10000);
  }
  for (i = 0; i < 6; i++) {
    pthread_create(&th[6 + i], 0, thr, (void*)i);
    if (rand() % 2)
      usleep(rand() % 10000);
  }
  usleep(100000);
  return 0;
}

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: net/sctp: use-after-free in __sctp_connect
  2016-10-19 12:25 Andrey Konovalov
@ 2016-10-19 16:57 ` Marcelo Ricardo Leitner
  2016-11-02 22:42   ` Andrey Konovalov
  0 siblings, 1 reply; 17+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-10-19 16:57 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Sasha Levin, Eric Dumazet, Dmitry Vyukov

On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
> Hi,
> 
> I've got the following error report while running the syzkaller fuzzer:
> 
> ==================================================================
> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
> ffff88006b1dc610

Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
So far I couldn't identify the reason.
"Good" to know it's still there, thanks for reporting it.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* net/sctp: use-after-free in __sctp_connect
@ 2016-10-19 12:25 Andrey Konovalov
  2016-10-19 16:57 ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Konovalov @ 2016-10-19 12:25 UTC (permalink / raw)
  To: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin,
	Eric Dumazet, Dmitry Vyukov

Hi,

I've got the following error report while running the syzkaller fuzzer:

==================================================================
BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
ffff88006b1dc610
Read of size 4 by task syz-executor/21837
CPU: 2 PID: 21837 Comm: syz-executor Not tainted 4.9.0-rc1+ #228
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffff8800393ef930 ffffffff81b474f4 ffff88003e80ed40 ffff88006b1dc568
 ffff88006b1dd6a0 ffff88006b1dc560 ffff8800393ef958 ffffffff8150b33c
 ffff8800393ef9e8 ffff88003e80ed40 ffff8800eb1dc610 ffff8800393ef9d8
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff81b474f4>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
 [<ffffffff8150b33c>] kasan_object_err+0x1c/0x70 mm/kasan/report.c:156
 [<     inline     >] print_address_description mm/kasan/report.c:194
 [<ffffffff8150b5d7>] kasan_report_error+0x1f7/0x4d0 mm/kasan/report.c:283
 [<     inline     >] kasan_report mm/kasan/report.c:303
 [<ffffffff8150b96e>] __asan_report_load4_noabort+0x3e/0x40
mm/kasan/report.c:323
 [<ffffffff8393457e>] __sctp_connect+0xabe/0xbf0 net/sctp/socket.c:1219
 [<ffffffff83934832>] __sctp_setsockopt_connectx+0x182/0x1b0
net/sctp/socket.c:1329
 [<     inline     >] sctp_setsockopt_connectx net/sctp/socket.c:1361
 [<ffffffff8393c1d9>] sctp_setsockopt+0x1009/0x3d70 net/sctp/socket.c:3813
 [<ffffffff82b770d6>] sock_common_setsockopt+0x96/0xd0 net/core/sock.c:2688
 [<     inline     >] SYSC_setsockopt net/socket.c:1742
 [<ffffffff82b740b4>] SyS_setsockopt+0x154/0x240 net/socket.c:1721
 [<ffffffff83fc0141>] entry_SYSCALL_64_fastpath+0x1f/0xc2
Object at ffff88006b1dc568, in cache kmalloc-4096 size: 4096
Allocated:
PID = 21837
 [  270.449111] [<ffffffff8107e2d6>] save_stack_trace+0x16/0x20
arch/x86/kernel/stacktrace.c:57
 [  270.449111] [<ffffffff8150a6a6>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [  270.449111] [<     inline     >] set_track mm/kasan/kasan.c:507
 [  270.449111] [<ffffffff8150a91b>] kasan_kmalloc+0xab/0xe0
mm/kasan/kasan.c:598
 [  270.449111] [<ffffffff8150604c>] kmem_cache_alloc_trace+0xec/0x270
mm/slub.c:2735
 [  270.449111] [<     inline     >] kmalloc include/linux/slab.h:490
 [  270.449111] [<     inline     >] kzalloc include/linux/slab.h:636
 [  270.449111] [<ffffffff838f2b2f>] sctp_association_new+0x6f/0x1f50
net/sctp/associola.c:303
 [  270.449111] [<ffffffff8393402a>] __sctp_connect+0x56a/0xbf0
net/sctp/socket.c:1163
 [  270.449111] [<ffffffff83934832>]
__sctp_setsockopt_connectx+0x182/0x1b0 net/sctp/socket.c:1329
 [  270.449111] [<     inline     >] sctp_setsockopt_connectx
net/sctp/socket.c:1361
 [  270.449111] [<ffffffff8393c1d9>] sctp_setsockopt+0x1009/0x3d70
net/sctp/socket.c:3813
 [  270.449111] [<ffffffff82b770d6>] sock_common_setsockopt+0x96/0xd0
net/core/sock.c:2688
 [  270.449111] [<     inline     >] SYSC_setsockopt net/socket.c:1742
 [  270.449111] [<ffffffff82b740b4>] SyS_setsockopt+0x154/0x240
net/socket.c:1721
 [  270.449111] [<ffffffff83fc0141>] entry_SYSCALL_64_fastpath+0x1f/0xc2
Freed:
PID = 21837
 [  270.449111] [<ffffffff8107e2d6>] save_stack_trace+0x16/0x20
arch/x86/kernel/stacktrace.c:57
 [  270.449111] [<ffffffff8150a6a6>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [  270.449111] [<     inline     >] set_track mm/kasan/kasan.c:507
 [  270.449111] [<ffffffff8150af03>] kasan_slab_free+0x73/0xc0
mm/kasan/kasan.c:571
 [  270.449111] [<     inline     >] slab_free_hook mm/slub.c:1352
 [  270.449111] [<     inline     >] slab_free_freelist_hook mm/slub.c:1374
 [  270.449111] [<     inline     >] slab_free mm/slub.c:2951
 [  270.449111] [<ffffffff815073e8>] kfree+0xe8/0x2b0 mm/slub.c:3871
 [  270.449111] [<     inline     >] sctp_association_destroy
net/sctp/associola.c:426
 [  270.449111] [<ffffffff838f56e5>] sctp_association_put+0x155/0x250
net/sctp/associola.c:866
 [  270.449111] [<ffffffff8392e213>] sctp_wait_for_connect+0x313/0x460
net/sctp/socket.c:7544
 [  270.449111] [<ffffffff8393443b>] __sctp_connect+0x97b/0xbf0
net/sctp/socket.c:1217
 [  270.449111] [<ffffffff83934832>]
__sctp_setsockopt_connectx+0x182/0x1b0 net/sctp/socket.c:1329
 [  270.449111] [<     inline     >] sctp_setsockopt_connectx
net/sctp/socket.c:1361
 [  270.449111] [<ffffffff8393c1d9>] sctp_setsockopt+0x1009/0x3d70
net/sctp/socket.c:3813
 [  270.449111] [<ffffffff82b770d6>] sock_common_setsockopt+0x96/0xd0
net/core/sock.c:2688
 [  270.449111] [<     inline     >] SYSC_setsockopt net/socket.c:1742
 [  270.449111] [<ffffffff82b740b4>] SyS_setsockopt+0x154/0x240
net/socket.c:1721
 [  270.449111] [<ffffffff83fc0141>] entry_SYSCALL_64_fastpath+0x1f/0xc2
Memory state around the buggy address:
 ffff88006b1dc500: fc fc fc fc fc fc fc fc fc fc fc fc fc fb fb fb
 ffff88006b1dc580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88006b1dc600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                         ^
 ffff88006b1dc680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88006b1dc700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

sctp_wait_for_connect ends up freeing asoc, which is later accessed to
read asoc->assoc_id.

On commit 1a1891d762d6e64daf07b5be4817e3fbb29e3c59 (Oct 18).

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-11-04 13:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-13  9:52 net/sctp: use-after-free in __sctp_connect Dmitry Vyukov
2016-01-14  1:37 ` YUAN Jia
2016-01-14  1:45   ` Marcelo Ricardo Leitner
2016-01-15 19:01 ` Marcelo Ricardo Leitner
2016-01-19 14:38   ` Vlad Yasevich
2016-01-21 17:18     ` Marcelo Ricardo Leitner
2016-01-21 17:37       ` Marcelo Ricardo Leitner
2016-10-19 12:25 Andrey Konovalov
2016-10-19 16:57 ` Marcelo Ricardo Leitner
2016-11-02 22:42   ` Andrey Konovalov
2016-11-03 17:11     ` Andrey Konovalov
2016-11-03 17:52       ` Marcelo Ricardo Leitner
2016-11-03 18:02         ` Andrey Konovalov
2016-11-03 18:35           ` Marcelo Ricardo Leitner
2016-11-03 18:45             ` Andrey Konovalov
2016-11-04 12:59             ` Neil Horman
2016-11-04 13:03               ` Marcelo Ricardo Leitner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).