Re: net: BUG in unix_notinflight

[Cong Wang]

On Mon, Mar 6, 2017 at 2:40 AM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> Now with a nice single-threaded C reproducer!

Excellent...

>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #define _GNU_SOURCE
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <stddef.h>
> #include <stdint.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
>
> void test()
> {
>   long r[54];
>   memset(r, -1, sizeof(r));
>   syscall(__NR_mmap, 0x20000000ul, 0xfff000ul, 0x3ul, 0x32ul, -1, 0);
>   r[1] = syscall(__NR_socketpair, 0x1ul, 0x5ul, 0x0ul, 0x20521ff8ul);
>   r[2] = *(uint32_t*)0x20521ff8;
>   r[3] = *(uint32_t*)0x20521ffc;
>   r[5] = syscall(__NR_open, "/dev/net/tun", 0x200000ul);
>   r[6] = syscall(__NR_socketpair, 0x1ul, 0x5ul, 0x0ul,
>                          0x20d85000ul, 0, 0, 0, 0, 0);
>   r[7] = *(uint32_t*)0x20d85000;
>   (*(uint64_t*)0x20000fc8 = (uint64_t)0x20000000);
>   (*(uint32_t*)0x20000fd0 = (uint32_t)0xa);
>   (*(uint64_t*)0x20000fd8 = (uint64_t)0x2005d000);
>   (*(uint64_t*)0x20000fe0 = (uint64_t)0x8);
>   (*(uint64_t*)0x20000fe8 = (uint64_t)0x20000ff0);
>   (*(uint64_t*)0x20000ff0 = (uint64_t)0x1);
>   (*(uint32_t*)0x20000ff8 = (uint32_t)0x0);
>   (*(uint16_t*)0x20000000 = (uint16_t)0x1);
>   memcpy((void*)0x20000002, "\x2e\x2f\x66\x69\x6c\x65\x30\x00", 8);
>   (*(uint64_t*)0x2005d000 = (uint64_t)0x20784f06);
>   (*(uint64_t*)0x2005d008 = (uint64_t)0x0);
>   (*(uint64_t*)0x2005d010 = (uint64_t)0x209a5f78);
>   (*(uint64_t*)0x2005d018 = (uint64_t)0x0);
>   (*(uint64_t*)0x2005d020 = (uint64_t)0x20ec3ffc);
>   (*(uint64_t*)0x2005d028 = (uint64_t)0x0);
>   (*(uint64_t*)0x2005d030 = (uint64_t)0x2057e000);
>   (*(uint64_t*)0x2005d038 = (uint64_t)0x0);
>   (*(uint64_t*)0x2005d040 = (uint64_t)0x200c9f9d);
>   (*(uint64_t*)0x2005d048 = (uint64_t)0x0);
>   (*(uint64_t*)0x2005d050 = (uint64_t)0x20331000);
>   (*(uint64_t*)0x2005d058 = (uint64_t)0x0);
>   (*(uint64_t*)0x2005d060 = (uint64_t)0x206a1f7b);
>   (*(uint64_t*)0x2005d068 = (uint64_t)0x0);
>   (*(uint64_t*)0x2005d070 = (uint64_t)0x20e7f000);
>   (*(uint64_t*)0x2005d078 = (uint64_t)0x0);
>   (*(uint64_t*)0x20000ff0 = (uint64_t)0x18);
>   (*(uint32_t*)0x20000ff8 = (uint32_t)0x1);
>   (*(uint32_t*)0x20000ffc = (uint32_t)0x1);
>   (*(uint32_t*)0x20001000 = r[5]);
>   (*(uint32_t*)0x20001004 = r[7]);
>   syscall(__NR_sendmsg, r[7], 0x20000fc8ul, 0x0ul);
>   (*(uint64_t*)0x20000fc8 = (uint64_t)0x20000000);
>   (*(uint32_t*)0x20000fd0 = (uint32_t)0x8);
>   (*(uint64_t*)0x20000fd8 = (uint64_t)0x20026000);
>   (*(uint64_t*)0x20000fe0 = (uint64_t)0x0);
>   (*(uint64_t*)0x20000fe8 = (uint64_t)0x20000ff0);
>   (*(uint64_t*)0x20000ff0 = (uint64_t)0x1);
>   (*(uint32_t*)0x20000ff8 = (uint32_t)0x0);
>   (*(uint16_t*)0x20000000 = (uint16_t)0x0);
>   (*(uint8_t*)0x20000002 = (uint8_t)0x0);
>   (*(uint32_t*)0x20000004 = (uint32_t)0x4e20);
>   (*(uint64_t*)0x20000ff0 = (uint64_t)0x18);
>   (*(uint32_t*)0x20000ff8 = (uint32_t)0x1);
>   (*(uint32_t*)0x20000ffc = (uint32_t)0x1);
>   (*(uint32_t*)0x20001000 = r[2]);
>   syscall(__NR_sendmsg, r[3], 0x20000fc8ul, 0x0ul);
> }
>
> int main()
> {
>   int i, pid, status;
>   for (i = 0; i < 4; i++) {
>     if (fork() == 0) {
>       for (;;) {
>         pid = fork();
>         if (pid == 0) {
>           test();
>           exit(0);
>         }
>         while (waitpid(pid, &status, __WALL) != pid) {}
>       }
>     }
>   }
>   sleep(1000000);
>   return 0;
> }
>
>
>
> New report from linux-next/c0b7b2b33bd17f7155956d0338ce92615da686c9
>
> ------------[ cut here ]------------
> kernel BUG at net/unix/garbage.c:149!
> invalid opcode: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 1806 Comm: syz-executor7 Not tainted 4.10.0-next-20170303+ #6
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 01/01/2011
> task: ffff880121c64740 task.stack: ffff88012c9e8000
> RIP: 0010:unix_notinflight+0x417/0x5d0 net/unix/garbage.c:149
> RSP: 0018:ffff88012c9ef0f8 EFLAGS: 00010297
> RAX: ffff880121c64740 RBX: 1ffff1002593de23 RCX: ffff8801c490c628
> RDX: 0000000000000000 RSI: 1ffff1002593de27 RDI: ffffffff8557e504
> RBP: ffff88012c9ef220 R08: 0000000000000001 R09: 0000000000000000
> R10: dffffc0000000000 R11: ffffed002593de55 R12: ffff8801c490c0c0
> R13: ffff88012c9ef1f8 R14: ffffffff85101620 R15: dffffc0000000000
> FS:  00000000013d3940(0000) GS:ffff8801dbe00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000001fd8cd8 CR3: 00000001cce69000 CR4: 00000000001426f0
> Call Trace:
>  unix_detach_fds.isra.23+0xfa/0x170 net/unix/af_unix.c:1490
>  unix_destruct_scm+0xf4/0x200 net/unix/af_unix.c:1499

The problem here is there is no lock protecting concurrent unix_detach_fds()
even though unix_notinflight() is already serialized, if we call
unix_notinflight()
twice on the same file pointer, we trigger this bug...

I don't know what is the right lock here to serialize it.