On Mon, Mar 6, 2017 at 11:34 PM, Cong Wang <xiyou.wangc...@gmail.com> wrote:
> On Mon, Mar 6, 2017 at 2:40 AM, Dmitry Vyukov <dvyu...@google.com> 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.
What exactly here needs to be protected?
1484 static void unix_detach_fds(struct scm_cookie *scm, struct sk_buff *skb)
1485 {
1486 int i;
1487
1488 scm->fp = UNIXCB(skb).fp;
1489 UNIXCB(skb).fp = NULL;
1490
1491 for (i = scm->fp->count-1; i >= 0; i--)
1492 unix_notinflight(scm->fp->user, scm->fp->fp[i]);
1493 }
Whole unix_notinflight happens under global unix_gc_lock.
Is it that 2 threads call unix_detach_fds for the same skb, and then
call unix_notinflight for the same fd twice?
