Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1593136 > unrolled thread

perf: use-after-free in perf_release

Back to article view | Back to linux.kernel

Contents

  perf: use-after-free in perf_release Dmitry Vyukov <dvyukov@google.com> - 2017-03-06 11:00 +0100
    Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-06 13:20 +0100
      Re: perf: use-after-free in perf_release Dmitry Vyukov <dvyukov@google.com> - 2017-03-06 13:20 +0100
        Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-06 13:30 +0100
          Re: perf: use-after-free in perf_release Dmitry Vyukov <dvyukov@google.com> - 2017-03-06 13:40 +0100
            Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-06 13:50 +0100
    Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-06 14:30 +0100
      Re: perf: use-after-free in perf_release Dmitry Vyukov <dvyukov@google.com> - 2017-03-06 14:40 +0100
        Re: perf: use-after-free in perf_release Dmitry Vyukov <dvyukov@google.com> - 2017-03-07 10:30 +0100
          Re: perf: use-after-free in perf_release Dmitry Vyukov <dvyukov@google.com> - 2017-03-07 10:50 +0100
            Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-07 12:50 +0100
          Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-07 11:40 +0100
        Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-07 10:50 +0100
      Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-07 14:20 +0100
        Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-07 15:10 +0100
      Re: perf: use-after-free in perf_release Oleg Nesterov <oleg@redhat.com> - 2017-03-07 15:10 +0100
        Re: perf: use-after-free in perf_release Dmitry Vyukov <dvyukov@google.com> - 2017-03-07 15:30 +0100
          Re: perf: use-after-free in perf_release Peter Zijlstra <peterz@infradead.org> - 2017-03-07 18:40 +0100
          Re: perf: use-after-free in perf_release Oleg Nesterov <oleg@redhat.com> - 2017-03-07 18:50 +0100

#1593136 — perf: use-after-free in perf_release

Hello,

I've got the following use-after-free report while running syzkaller
fuzzer on 86292b33d4b79ee03e2f43ea0381ef85f077c760. Note that the task
is freed right in copy_process due to some error, but it's referenced
by another thread in perf subsystem.

==================================================================
BUG: KASAN: use-after-free in atomic_dec_and_test
arch/x86/include/asm/atomic.h:123 [inline] at addr ffff880079c30158
BUG: KASAN: use-after-free in put_task_struct
include/linux/sched/task.h:93 [inline] at addr ffff880079c30158
BUG: KASAN: use-after-free in put_ctx+0xcf/0x110
kernel/events/core.c:1131 at addr ffff880079c30158
Write of size 4 by task syz-executor6/25698
CPU: 2 PID: 25698 Comm: syz-executor6 Not tainted 4.10.0+ #302
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:16 [inline]
 dump_stack+0x2fb/0x3fd lib/dump_stack.c:52
 kasan_object_err+0x1c/0x90 mm/kasan/report.c:166
 print_address_description mm/kasan/report.c:208 [inline]
 kasan_report_error mm/kasan/report.c:292 [inline]
 kasan_report.part.2+0x1b0/0x460 mm/kasan/report.c:314
 kasan_report+0x21/0x30 mm/kasan/report.c:301
 check_memory_region_inline mm/kasan/kasan.c:326 [inline]
 check_memory_region+0x139/0x190 mm/kasan/kasan.c:333
 kasan_check_write+0x14/0x20 mm/kasan/kasan.c:344
 atomic_dec_and_test arch/x86/include/asm/atomic.h:123 [inline]
 put_task_struct include/linux/sched/task.h:93 [inline]
 put_ctx+0xcf/0x110 kernel/events/core.c:1131
 perf_event_release_kernel+0x3ad/0xc90 kernel/events/core.c:4322
 perf_release+0x37/0x50 kernel/events/core.c:4338
 __fput+0x332/0x800 fs/file_table.c:209
 ____fput+0x15/0x20 fs/file_table.c:245
 task_work_run+0x197/0x260 kernel/task_work.c:116
 exit_task_work include/linux/task_work.h:21 [inline]
 do_exit+0xb38/0x29c0 kernel/exit.c:880
 do_group_exit+0x149/0x420 kernel/exit.c:984
 get_signal+0x7e0/0x1820 kernel/signal.c:2318
 do_signal+0xd2/0x2190 arch/x86/kernel/signal.c:808
 exit_to_usermode_loop+0x200/0x2a0 arch/x86/entry/common.c:157
 syscall_return_slowpath arch/x86/entry/common.c:191 [inline]
 do_syscall_64+0x6fc/0x930 arch/x86/entry/common.c:286
 entry_SYSCALL64_slow_path+0x25/0x25
RIP: 0033:0x4458d9
RSP: 002b:00007f3f07187cf8 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: fffffffffffffe00 RBX: 00000000007080c8 RCX: 00000000004458d9
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000007080c8
RBP: 00000000007080a8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007f3f071889c0 R15: 00007f3f07188700
Object at ffff880079c30140, in cache task_struct size: 5376
Allocated:
PID = 25681
 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:513
 set_track mm/kasan/kasan.c:525 [inline]
 kasan_kmalloc+0xaa/0xd0 mm/kasan/kasan.c:616
 kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:555
 kmem_cache_alloc_node+0x122/0x6f0 mm/slab.c:3662
 alloc_task_struct_node kernel/fork.c:153 [inline]
 dup_task_struct kernel/fork.c:495 [inline]
 copy_process.part.38+0x19c8/0x4aa0 kernel/fork.c:1560
 copy_process kernel/fork.c:1531 [inline]
 _do_fork+0x200/0x1010 kernel/fork.c:1994
 SYSC_clone kernel/fork.c:2104 [inline]
 SyS_clone+0x37/0x50 kernel/fork.c:2098
 do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:281
 return_from_SYSCALL_64+0x0/0x7a
Freed:
PID = 25681
 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:513
 set_track mm/kasan/kasan.c:525 [inline]
 kasan_slab_free+0x6f/0xb0 mm/kasan/kasan.c:589
 __cache_free mm/slab.c:3514 [inline]
 kmem_cache_free+0x71/0x240 mm/slab.c:3774
 free_task_struct kernel/fork.c:158 [inline]
 free_task+0x151/0x1d0 kernel/fork.c:370
 copy_process.part.38+0x18e5/0x4aa0 kernel/fork.c:1931
 copy_process kernel/fork.c:1531 [inline]
 _do_fork+0x200/0x1010 kernel/fork.c:1994
 SYSC_clone kernel/fork.c:2104 [inline]
 SyS_clone+0x37/0x50 kernel/fork.c:2098
 do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:281
 return_from_SYSCALL_64+0x0/0x7a

[toc] | [next] | [standalone]

#1593256

On Mon, Mar 06, 2017 at 10:57:07AM +0100, Dmitry Vyukov wrote:
> Hello,
> 
> I've got the following use-after-free report while running syzkaller
> fuzzer on 86292b33d4b79ee03e2f43ea0381ef85f077c760. Note that the task
> is freed right in copy_process due to some error, but it's referenced
> by another thread in perf subsystem.

Weird... you don't happen to have a reproduction case available?

[toc] | [prev] | [next] | [standalone]

#1593261

On Mon, Mar 6, 2017 at 1:13 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 06, 2017 at 10:57:07AM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> I've got the following use-after-free report while running syzkaller
>> fuzzer on 86292b33d4b79ee03e2f43ea0381ef85f077c760. Note that the task
>> is freed right in copy_process due to some error, but it's referenced
>> by another thread in perf subsystem.
>
> Weird... you don't happen to have a reproduction case available?


Unfortunately no. I've looked at both logs that I have and there are
no memory allocation failures preceding the crash (however maybe
somebody used NOWARN?). But probably if you inject an error into
copy_process somewhere after perf_event_init_task, it should reproduce
the bug with KASAN I think.

[toc] | [prev] | [next] | [standalone]

#1593271

On Mon, Mar 06, 2017 at 01:17:42PM +0100, Dmitry Vyukov wrote:
> On Mon, Mar 6, 2017 at 1:13 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Mar 06, 2017 at 10:57:07AM +0100, Dmitry Vyukov wrote:
> >> Hello,
> >>
> >> I've got the following use-after-free report while running syzkaller
> >> fuzzer on 86292b33d4b79ee03e2f43ea0381ef85f077c760. Note that the task
> >> is freed right in copy_process due to some error, but it's referenced
> >> by another thread in perf subsystem.
> >
> > Weird... you don't happen to have a reproduction case available?
> 
> 
> Unfortunately no. I've looked at both logs that I have and there are
> no memory allocation failures preceding the crash (however maybe
> somebody used NOWARN?). But probably if you inject an error into
> copy_process somewhere after perf_event_init_task, it should reproduce
> the bug with KASAN I think.

I'll try. Thanks!

[toc] | [prev] | [next] | [standalone]

#1593278

[Multipart message — attachments visible in raw view] — view raw

On Mon, Mar 6, 2017 at 1:23 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 06, 2017 at 01:17:42PM +0100, Dmitry Vyukov wrote:
>> On Mon, Mar 6, 2017 at 1:13 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Mon, Mar 06, 2017 at 10:57:07AM +0100, Dmitry Vyukov wrote:
>> >> Hello,
>> >>
>> >> I've got the following use-after-free report while running syzkaller
>> >> fuzzer on 86292b33d4b79ee03e2f43ea0381ef85f077c760. Note that the task
>> >> is freed right in copy_process due to some error, but it's referenced
>> >> by another thread in perf subsystem.
>> >
>> > Weird... you don't happen to have a reproduction case available?
>>
>>
>> Unfortunately no. I've looked at both logs that I have and there are
>> no memory allocation failures preceding the crash (however maybe
>> somebody used NOWARN?). But probably if you inject an error into
>> copy_process somewhere after perf_event_init_task, it should reproduce
>> the bug with KASAN I think.
>
> I'll try. Thanks!


I think you will also need the attached patch. It seems that it was
found due to it. Going to send it out soon.

[toc] | [prev] | [next] | [standalone]

#1593282

On Mon, Mar 06, 2017 at 01:27:41PM +0100, Dmitry Vyukov wrote:

> I think you will also need the attached patch. It seems that it was
> found due to it. Going to send it out soon.

Yuck, that's nasty. Although I don't see an alternative there.

You might also want to do the bitops, they suffer the same problem.

[toc] | [prev] | [next] | [standalone]

#1593306

On Mon, Mar 06, 2017 at 10:57:07AM +0100, Dmitry Vyukov wrote:

> ==================================================================
> BUG: KASAN: use-after-free in atomic_dec_and_test
> arch/x86/include/asm/atomic.h:123 [inline] at addr ffff880079c30158
> BUG: KASAN: use-after-free in put_task_struct
> include/linux/sched/task.h:93 [inline] at addr ffff880079c30158
> BUG: KASAN: use-after-free in put_ctx+0xcf/0x110

FWIW, this output is very confusing, is this a result of your
post-processing replicating the line for every 'inlined' part?

> kernel/events/core.c:1131 at addr ffff880079c30158
> Write of size 4 by task syz-executor6/25698

>  atomic_dec_and_test arch/x86/include/asm/atomic.h:123 [inline]
>  put_task_struct include/linux/sched/task.h:93 [inline]
>  put_ctx+0xcf/0x110 kernel/events/core.c:1131
>  perf_event_release_kernel+0x3ad/0xc90 kernel/events/core.c:4322
>  perf_release+0x37/0x50 kernel/events/core.c:4338
>  __fput+0x332/0x800 fs/file_table.c:209
>  ____fput+0x15/0x20 fs/file_table.c:245
>  task_work_run+0x197/0x260 kernel/task_work.c:116
>  exit_task_work include/linux/task_work.h:21 [inline]
>  do_exit+0xb38/0x29c0 kernel/exit.c:880
>  do_group_exit+0x149/0x420 kernel/exit.c:984
>  get_signal+0x7e0/0x1820 kernel/signal.c:2318
>  do_signal+0xd2/0x2190 arch/x86/kernel/signal.c:808
>  exit_to_usermode_loop+0x200/0x2a0 arch/x86/entry/common.c:157
>  syscall_return_slowpath arch/x86/entry/common.c:191 [inline]
>  do_syscall_64+0x6fc/0x930 arch/x86/entry/common.c:286
>  entry_SYSCALL64_slow_path+0x25/0x25

So this is fput()..


> Freed:
> PID = 25681
>  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>  set_track mm/kasan/kasan.c:525 [inline]
>  kasan_slab_free+0x6f/0xb0 mm/kasan/kasan.c:589
>  __cache_free mm/slab.c:3514 [inline]
>  kmem_cache_free+0x71/0x240 mm/slab.c:3774
>  free_task_struct kernel/fork.c:158 [inline]
>  free_task+0x151/0x1d0 kernel/fork.c:370
>  copy_process.part.38+0x18e5/0x4aa0 kernel/fork.c:1931
>  copy_process kernel/fork.c:1531 [inline]
>  _do_fork+0x200/0x1010 kernel/fork.c:1994
>  SYSC_clone kernel/fork.c:2104 [inline]
>  SyS_clone+0x37/0x50 kernel/fork.c:2098
>  do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:281
>  return_from_SYSCALL_64+0x0/0x7a

and this is a failed fork().


However, inherited events don't have a filedesc to fput(), and
similarly, a task that fails for has never been visible to attach a perf
event to because it never hits the pid-hash.

Or so it is assumed.

I'm forever getting lost in the PID code. Oleg, is there any way
find_task_by_vpid() can return a task that can still fail fork() ?

[toc] | [prev] | [next] | [standalone]

#1593316

On Mon, Mar 6, 2017 at 2:14 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 06, 2017 at 10:57:07AM +0100, Dmitry Vyukov wrote:
>
>> ==================================================================
>> BUG: KASAN: use-after-free in atomic_dec_and_test
>> arch/x86/include/asm/atomic.h:123 [inline] at addr ffff880079c30158
>> BUG: KASAN: use-after-free in put_task_struct
>> include/linux/sched/task.h:93 [inline] at addr ffff880079c30158
>> BUG: KASAN: use-after-free in put_ctx+0xcf/0x110
>
> FWIW, this output is very confusing, is this a result of your
> post-processing replicating the line for every 'inlined' part?


Yes.
We probably should not do this inlining in the header line. But the
problem is that it is very difficult to understand that it is a header
line in general.


>> kernel/events/core.c:1131 at addr ffff880079c30158
>> Write of size 4 by task syz-executor6/25698
>
>>  atomic_dec_and_test arch/x86/include/asm/atomic.h:123 [inline]
>>  put_task_struct include/linux/sched/task.h:93 [inline]
>>  put_ctx+0xcf/0x110 kernel/events/core.c:1131
>>  perf_event_release_kernel+0x3ad/0xc90 kernel/events/core.c:4322
>>  perf_release+0x37/0x50 kernel/events/core.c:4338
>>  __fput+0x332/0x800 fs/file_table.c:209
>>  ____fput+0x15/0x20 fs/file_table.c:245
>>  task_work_run+0x197/0x260 kernel/task_work.c:116
>>  exit_task_work include/linux/task_work.h:21 [inline]
>>  do_exit+0xb38/0x29c0 kernel/exit.c:880
>>  do_group_exit+0x149/0x420 kernel/exit.c:984
>>  get_signal+0x7e0/0x1820 kernel/signal.c:2318
>>  do_signal+0xd2/0x2190 arch/x86/kernel/signal.c:808
>>  exit_to_usermode_loop+0x200/0x2a0 arch/x86/entry/common.c:157
>>  syscall_return_slowpath arch/x86/entry/common.c:191 [inline]
>>  do_syscall_64+0x6fc/0x930 arch/x86/entry/common.c:286
>>  entry_SYSCALL64_slow_path+0x25/0x25
>
> So this is fput()..
>
>
>> Freed:
>> PID = 25681
>>  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>>  save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>>  set_track mm/kasan/kasan.c:525 [inline]
>>  kasan_slab_free+0x6f/0xb0 mm/kasan/kasan.c:589
>>  __cache_free mm/slab.c:3514 [inline]
>>  kmem_cache_free+0x71/0x240 mm/slab.c:3774
>>  free_task_struct kernel/fork.c:158 [inline]
>>  free_task+0x151/0x1d0 kernel/fork.c:370
>>  copy_process.part.38+0x18e5/0x4aa0 kernel/fork.c:1931
>>  copy_process kernel/fork.c:1531 [inline]
>>  _do_fork+0x200/0x1010 kernel/fork.c:1994
>>  SYSC_clone kernel/fork.c:2104 [inline]
>>  SyS_clone+0x37/0x50 kernel/fork.c:2098
>>  do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:281
>>  return_from_SYSCALL_64+0x0/0x7a
>
> and this is a failed fork().
>
>
> However, inherited events don't have a filedesc to fput(), and
> similarly, a task that fails for has never been visible to attach a perf
> event to because it never hits the pid-hash.
>
> Or so it is assumed.
>
> I'm forever getting lost in the PID code. Oleg, is there any way
> find_task_by_vpid() can return a task that can still fail fork() ?


FWIW here are 2 syzkaller programs that triggered the bug:
https://gist.githubusercontent.com/dvyukov/d67f980050589775237a7fbdff226bec/raw/4bca72861cb2ede64059b6dad403e19f425a361f/gistfile1.txt
They look very similar, so most likely they are a mutation of the same
program. Which may suggest that there is something in that program
that provokes the bug. Note that the calls in these programs are
executed potentially in multiple threads. But at least it can give
some idea wrt e.g. flags passed to perf_event_open.

[toc] | [prev] | [next] | [standalone]

#1594039

On Tue, Mar 7, 2017 at 10:08 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Mar 06, 2017 at 02:34:50PM +0100, Dmitry Vyukov wrote:
>> FWIW here are 2 syzkaller programs that triggered the bug:
>> https://gist.githubusercontent.com/dvyukov/d67f980050589775237a7fbdff226bec/raw/4bca72861cb2ede64059b6dad403e19f425a361f/gistfile1.txt
>
> Hurm, previously your gistfile thingies were actual C, but this thing is
> gibberish. How do I run it?

The same way we did it here:
https://groups.google.com/d/msg/syzkaller/MHXa-o8foyc/yrGfDOrwAQAJ
This will run it in infinite loop in 10 parallel processes:
./syz-execprog -repeat=0 -procs=10 -sandbox=namespace gistfile1.txt
-sandbox=namespace will require CONFIG_USER_NS=y, I am not sure if it
is actually required, but that's how bots triggered it. You can do
-sandbox=none as well.

[toc] | [prev] | [next] | [standalone]

#1594050

On Tue, Mar 7, 2017 at 10:37 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Mar 07, 2017 at 10:26:20AM +0100, Dmitry Vyukov wrote:
>> On Tue, Mar 7, 2017 at 10:08 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Mon, Mar 06, 2017 at 02:34:50PM +0100, Dmitry Vyukov wrote:
>> >> FWIW here are 2 syzkaller programs that triggered the bug:
>> >> https://gist.githubusercontent.com/dvyukov/d67f980050589775237a7fbdff226bec/raw/4bca72861cb2ede64059b6dad403e19f425a361f/gistfile1.txt
>> >
>> > Hurm, previously your gistfile thingies were actual C, but this thing is
>> > gibberish. How do I run it?
>>
>> The same way we did it here:
>> https://groups.google.com/d/msg/syzkaller/MHXa-o8foyc/yrGfDOrwAQAJ
>
> Oh right, completely forgot about that. The last gistfile stuff I found
> in my history were actual C files.
>
>> This will run it in infinite loop in 10 parallel processes:
>> ./syz-execprog -repeat=0 -procs=10 -sandbox=namespace gistfile1.txt
>> -sandbox=namespace will require CONFIG_USER_NS=y, I am not sure if it
>> is actually required, but that's how bots triggered it. You can do
>> -sandbox=none as well.
>
> I still have an ancient syzcaller; -sandbox doesn't exist and I needed
> to add -executor=bin/syz-executor but now it appears to run.
>
> I'll go up procs, 10 is somewhat low I feel.
>
>
> ---
>
> root@ivb-ep:~/gopath/src/github.com/google/syzkaller# ./bin/syz-execprog -repeat=0 -procs=10 -executor=bin/syz-executor gistfile2.txt
> 2017/03/07 10:35:14 parsed 2 programs
> 2017/03/07 10:35:14 executed 0 programs
> result: failed=false hanged=false err=executor is not serving
>
> 2017/03/07 10:36:14 executed 10 programs
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving
>
> result: failed=false hanged=false err=executor is not serving


An old syzkaller may not understand part of syscalls in the program
and silently drop them, you need a new one.

Here is a straightforward conversion of the syzkaller program to C
(with/without namespace sandbox):

https://gist.githubusercontent.com/dvyukov/b6540bed50b7da1dff3d7373ba570c77/raw/fd5f2f3aaa52b70b2bb9f114cf8a3226d8a30960/gistfile1.txt
https://gist.githubusercontent.com/dvyukov/dbd8ec38bcb50df4bdc95210d4247b09/raw/f9cbb5e17cd4ff4a7a7881c97dea7d6cd6dd8bf1/gistfile1.txt

That's also with -procs=10, you can change number of procs in main funciton.

But I wasn't able to reproduce the crash using these programs (neither
the syzkaller program), that's why I did not provide it all initially.

[toc] | [prev] | [next] | [standalone]

#1594148

On Tue, Mar 07, 2017 at 10:43:32AM +0100, Dmitry Vyukov wrote:

> An old syzkaller may not understand part of syscalls in the program
> and silently drop them, you need a new one.

That's yucky semantics, better to at least warn on that occasion.

> Here is a straightforward conversion of the syzkaller program to C
> (with/without namespace sandbox):
> 
> https://gist.githubusercontent.com/dvyukov/b6540bed50b7da1dff3d7373ba570c77/raw/fd5f2f3aaa52b70b2bb9f114cf8a3226d8a30960/gistfile1.txt
> https://gist.githubusercontent.com/dvyukov/dbd8ec38bcb50df4bdc95210d4247b09/raw/f9cbb5e17cd4ff4a7a7881c97dea7d6cd6dd8bf1/gistfile1.txt

Thanks!

> That's also with -procs=10, you can change number of procs in main funciton.
> 
> But I wasn't able to reproduce the crash using these programs (neither
> the syzkaller program), that's why I did not provide it all initially.

Right, I'll run them while at the same time trying to see what it is
they're doing to find clues.

[toc] | [prev] | [next] | [standalone]

#1594091

On Tue, Mar 07, 2017 at 10:26:20AM +0100, Dmitry Vyukov wrote:
> On Tue, Mar 7, 2017 at 10:08 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Mar 06, 2017 at 02:34:50PM +0100, Dmitry Vyukov wrote:
> >> FWIW here are 2 syzkaller programs that triggered the bug:
> >> https://gist.githubusercontent.com/dvyukov/d67f980050589775237a7fbdff226bec/raw/4bca72861cb2ede64059b6dad403e19f425a361f/gistfile1.txt
> >
> > Hurm, previously your gistfile thingies were actual C, but this thing is
> > gibberish. How do I run it?
> 
> The same way we did it here:
> https://groups.google.com/d/msg/syzkaller/MHXa-o8foyc/yrGfDOrwAQAJ

Oh right, completely forgot about that. The last gistfile stuff I found
in my history were actual C files.

> This will run it in infinite loop in 10 parallel processes:
> ./syz-execprog -repeat=0 -procs=10 -sandbox=namespace gistfile1.txt
> -sandbox=namespace will require CONFIG_USER_NS=y, I am not sure if it
> is actually required, but that's how bots triggered it. You can do
> -sandbox=none as well.

I still have an ancient syzcaller; -sandbox doesn't exist and I needed
to add -executor=bin/syz-executor but now it appears to run.

I'll go up procs, 10 is somewhat low I feel.


---

root@ivb-ep:~/gopath/src/github.com/google/syzkaller# ./bin/syz-execprog -repeat=0 -procs=10 -executor=bin/syz-executor gistfile2.txt
2017/03/07 10:35:14 parsed 2 programs
2017/03/07 10:35:14 executed 0 programs
result: failed=false hanged=false err=executor is not serving

2017/03/07 10:36:14 executed 10 programs
result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

result: failed=false hanged=false err=executor is not serving

[toc] | [prev] | [next] | [standalone]

#1594048

On Mon, Mar 06, 2017 at 02:34:50PM +0100, Dmitry Vyukov wrote:
> FWIW here are 2 syzkaller programs that triggered the bug:
> https://gist.githubusercontent.com/dvyukov/d67f980050589775237a7fbdff226bec/raw/4bca72861cb2ede64059b6dad403e19f425a361f/gistfile1.txt

Hurm, previously your gistfile thingies were actual C, but this thing is
gibberish. How do I run it?

[toc] | [prev] | [next] | [standalone]

#1594228

On Mon, Mar 06, 2017 at 02:14:59PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 06, 2017 at 10:57:07AM +0100, Dmitry Vyukov wrote:
> 
> > ==================================================================
> > BUG: KASAN: use-after-free in atomic_dec_and_test
> > arch/x86/include/asm/atomic.h:123 [inline] at addr ffff880079c30158
> > BUG: KASAN: use-after-free in put_task_struct
> > include/linux/sched/task.h:93 [inline] at addr ffff880079c30158
> > BUG: KASAN: use-after-free in put_ctx+0xcf/0x110
> 
> FWIW, this output is very confusing, is this a result of your
> post-processing replicating the line for every 'inlined' part?
> 
> > kernel/events/core.c:1131 at addr ffff880079c30158
> > Write of size 4 by task syz-executor6/25698
> 
> >  atomic_dec_and_test arch/x86/include/asm/atomic.h:123 [inline]
> >  put_task_struct include/linux/sched/task.h:93 [inline]
> >  put_ctx+0xcf/0x110 kernel/events/core.c:1131
> >  perf_event_release_kernel+0x3ad/0xc90 kernel/events/core.c:4322
> >  perf_release+0x37/0x50 kernel/events/core.c:4338
> >  __fput+0x332/0x800 fs/file_table.c:209
> >  ____fput+0x15/0x20 fs/file_table.c:245
> >  task_work_run+0x197/0x260 kernel/task_work.c:116
> >  exit_task_work include/linux/task_work.h:21 [inline]
> >  do_exit+0xb38/0x29c0 kernel/exit.c:880
> >  do_group_exit+0x149/0x420 kernel/exit.c:984
> >  get_signal+0x7e0/0x1820 kernel/signal.c:2318
> >  do_signal+0xd2/0x2190 arch/x86/kernel/signal.c:808
> >  exit_to_usermode_loop+0x200/0x2a0 arch/x86/entry/common.c:157
> >  syscall_return_slowpath arch/x86/entry/common.c:191 [inline]
> >  do_syscall_64+0x6fc/0x930 arch/x86/entry/common.c:286
> >  entry_SYSCALL64_slow_path+0x25/0x25
> 
> So this is fput()..
> 
> 
> > Freed:
> > PID = 25681
> >  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> >  save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> >  set_track mm/kasan/kasan.c:525 [inline]
> >  kasan_slab_free+0x6f/0xb0 mm/kasan/kasan.c:589
> >  __cache_free mm/slab.c:3514 [inline]
> >  kmem_cache_free+0x71/0x240 mm/slab.c:3774
> >  free_task_struct kernel/fork.c:158 [inline]
> >  free_task+0x151/0x1d0 kernel/fork.c:370
> >  copy_process.part.38+0x18e5/0x4aa0 kernel/fork.c:1931
> >  copy_process kernel/fork.c:1531 [inline]
> >  _do_fork+0x200/0x1010 kernel/fork.c:1994
> >  SYSC_clone kernel/fork.c:2104 [inline]
> >  SyS_clone+0x37/0x50 kernel/fork.c:2098
> >  do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:281
> >  return_from_SYSCALL_64+0x0/0x7a
> 
> and this is a failed fork().
> 
> 
> However, inherited events don't have a filedesc to fput(), and
> similarly, a task that fails for has never been visible to attach a perf
> event to because it never hits the pid-hash.
> 
> Or so it is assumed.
> 
> I'm forever getting lost in the PID code. Oleg, is there any way
> find_task_by_vpid() can return a task that can still fail fork() ?

So I _think_ find_task_by_vpid() can return an already dead task; and
we'll happily increase task->usage.

Dmitry; I have no idea how easy it is for you to reproduce the thing;
but so far I've not had much success. Could you perhaps stick the below
in?

Once we convert task_struct to refcount_t that should generate a WARN of
its own I suppose.

---

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 000fdb2..612d652 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -763,6 +763,7 @@ struct perf_event_context {
 #ifdef CONFIG_CGROUP_PERF
 	int				nr_cgroups;	 /* cgroup evts */
 #endif
+	int				switches;
 	void				*task_ctx_data; /* pmu specific data */
 	struct rcu_head			rcu_head;
 };
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 6f41548f..6455b7a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2902,6 +2902,8 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn,
 	if (!parent && !next_parent)
 		goto unlock;
 
+	ctx->switches++;
+
 	if (next_parent == ctx || next_ctx == parent || next_parent == parent) {
 		/*
 		 * Looks like the two contexts are clones, so we might be
@@ -3780,6 +3782,12 @@ find_lively_task_by_vpid(pid_t vpid)
 		task = current;
 	else
 		task = find_task_by_vpid(vpid);
+
+	if (task) {
+		if (WARN_ON_ONCE(task->flags & PF_EXITING))
+			task = NULL;
+	}
+
 	if (task)
 		get_task_struct(task);
 	rcu_read_unlock();
@@ -10432,6 +10440,10 @@ void perf_event_free_task(struct task_struct *task)
 
 		mutex_unlock(&ctx->mutex);
 
+		WARN_ON_ONCE(ctx->switches);
+		WARN_ON_ONCE(atomic_read(&ctx->refcount) != 1);
+		WARN_ON_ONCE(ctx->task != task);
+
 		put_ctx(ctx);
 	}
 }

[toc] | [prev] | [next] | [standalone]

#1594254

On Tue, Mar 07, 2017 at 02:16:49PM +0100, Peter Zijlstra wrote:
> So I _think_ find_task_by_vpid() can return an already dead task; and
> we'll happily increase task->usage.

Hurm, so find_get_context() already does the PF_EXITING test. And then
the put_ctx would've been from find_get_context(), not fput().

So still puzzled.

[toc] | [prev] | [next] | [standalone]

#1594258

On 03/06, Peter Zijlstra wrote:
>
> and this is a failed fork().
>
>
> However, inherited events don't have a filedesc to fput(), and
> similarly, a task that fails for has never been visible to attach a perf
> event to because it never hits the pid-hash.

Yes, it is not visible to find_task_by_vpid() until copy_process() does
attach_pid(PIDTYPE_PID), and copy_process() can't fail after that.

Oleg.

[toc] | [prev] | [next] | [standalone]

#1594271

On Tue, Mar 7, 2017 at 3:04 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 03/06, Peter Zijlstra wrote:
>>
>> and this is a failed fork().
>>
>>
>> However, inherited events don't have a filedesc to fput(), and
>> similarly, a task that fails for has never been visible to attach a perf
>> event to because it never hits the pid-hash.
>
> Yes, it is not visible to find_task_by_vpid() until copy_process() does
> attach_pid(PIDTYPE_PID), and copy_process() can't fail after that.


I would what is that that is failed in copy_process. Could it be
perf_event_init_task itself? Maybe it leaves a pointer to p in some
shared state on some error conditions?

[toc] | [prev] | [next] | [standalone]

#1594458

On Tue, Mar 07, 2017 at 05:51:32PM +0100, Oleg Nesterov wrote:
> On 03/07, Dmitry Vyukov wrote:

> > I would what is that that is failed in copy_process. Could it be
> > perf_event_init_task itself? Maybe it leaves a pointer to p in some
> > shared state on some error conditions?
> 
> I am looking at perf_event_init_task() too and I can't understand the
> error handling...
> 
> perf_event_init_context() can return success even if inherit_task_group() in
> the first list_for_each_entry(pinned_groups) fails, "ret" will be overwritten
> by the 2nd list_for_each_entry(flexible_groups) loop. "inherited_all" should
> be cleared, still this looks confusing at least.
> 
> inherit_event() returns NULL under is_orphaned_event() check, not ERR_PTR().
> Is it correct?

Urgh, there was something tricky there, but I cannot remember, and it
seems we didn't put a comment in either :/

Alexander, can you remember?

But yes, this all looks a tad dodgy, I'll try and have a look, but I
feel like I'm coming down with something :-(

[toc] | [prev] | [next] | [standalone]

#1594462

On 03/07, Dmitry Vyukov wrote:
>
> On Tue, Mar 7, 2017 at 3:04 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> > On 03/06, Peter Zijlstra wrote:
> >>
> >> and this is a failed fork().
> >>
> >>
> >> However, inherited events don't have a filedesc to fput(), and
> >> similarly, a task that fails for has never been visible to attach a perf
> >> event to because it never hits the pid-hash.
> >
> > Yes, it is not visible to find_task_by_vpid() until copy_process() does
> > attach_pid(PIDTYPE_PID), and copy_process() can't fail after that.
>
>
> I would what is that that is failed in copy_process. Could it be
> perf_event_init_task itself? Maybe it leaves a pointer to p in some
> shared state on some error conditions?

I am looking at perf_event_init_task() too and I can't understand the
error handling...

perf_event_init_context() can return success even if inherit_task_group() in
the first list_for_each_entry(pinned_groups) fails, "ret" will be overwritten
by the 2nd list_for_each_entry(flexible_groups) loop. "inherited_all" should
be cleared, still this looks confusing at least.

inherit_event() returns NULL under is_orphaned_event() check, not ERR_PTR().
Is it correct?

Oleg.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web