Re: [syzbot] KASAN: use-after-free Read in put_pmu_ctx

[Chengming Zhou <zhouchengming@bytedance.com>]

On 2022/12/19 22:40, Peter Zijlstra wrote:
> On Mon, Dec 19, 2022 at 12:04:43AM -0800, syzbot wrote:
>> Hello,
>>
>> syzbot found the following issue on:
>>
>> HEAD commit:    13e3c7793e2f Merge tag 'for-netdev' of https://git.kernel...
>> git tree:       bpf
>> console+strace: https://syzkaller.appspot.com/x/log.txt?x=177df7e0480000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=b0e91ad4b5f69c47
>> dashboard link: https://syzkaller.appspot.com/bug?extid=b8e8c01c8ade4fe6e48f
>> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15e87100480000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16ceeb13880000
>>
>> Downloadable assets:
>> disk image: https://storage.googleapis.com/syzbot-assets/373a99daa295/disk-13e3c779.raw.xz
>> vmlinux: https://storage.googleapis.com/syzbot-assets/7fa71ed0fe17/vmlinux-13e3c779.xz
>> kernel image: https://storage.googleapis.com/syzbot-assets/2842ad5c698b/bzImage-13e3c779.xz
>>
>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> Reported-by: syzbot+b8e8c01c8ade4fe6e48f@syzkaller.appspotmail.com
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in __lock_acquire+0x3ee7/0x56d0 kernel/locking/lockdep.c:4925
>> Read of size 8 at addr ffff8880237d6018 by task syz-executor287/8300
>>
>> CPU: 0 PID: 8300 Comm: syz-executor287 Not tainted 6.1.0-syzkaller-09661-g13e3c7793e2f #0
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/26/2022
>> Call Trace:
>>  <TASK>
>>  __dump_stack lib/dump_stack.c:88 [inline]
>>  dump_stack_lvl+0xd1/0x138 lib/dump_stack.c:106
>>  print_address_description mm/kasan/report.c:284 [inline]
>>  print_report+0x15e/0x45d mm/kasan/report.c:395
>>  kasan_report+0xbf/0x1f0 mm/kasan/report.c:495
>>  __lock_acquire+0x3ee7/0x56d0 kernel/locking/lockdep.c:4925
>>  lock_acquire kernel/locking/lockdep.c:5668 [inline]
>>  lock_acquire+0x1e3/0x630 kernel/locking/lockdep.c:5633
>>  __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>>  _raw_spin_lock_irqsave+0x3d/0x60 kernel/locking/spinlock.c:162
>>  put_pmu_ctx kernel/events/core.c:4913 [inline]
>>  put_pmu_ctx+0xad/0x390 kernel/events/core.c:4893
>>  _free_event+0x3c5/0x13d0 kernel/events/core.c:5196
>>  free_event+0x58/0xc0 kernel/events/core.c:5224
>>  __do_sys_perf_event_open+0x66d/0x2980 kernel/events/core.c:12701
>>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>>  do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
>>  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> 
> Does this help?
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index e47914ac8732..bbff551783e1 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -12689,7 +12689,8 @@ SYSCALL_DEFINE5(perf_event_open,
>  	return event_fd;
>  
>  err_context:
> -	/* event->pmu_ctx freed by free_event() */
> +	put_pmu_ctx(event->pmu_ctx);
> +	event->pmu_ctx = NULL; /* _free_event() */
>  err_locked:
>  	mutex_unlock(&ctx->mutex);
>  	perf_unpin_context(ctx);

Tested-by: Chengming Zhou <zhouchengming@bytedance.com>

While reviewing the code, I found perf_event_create_kernel_counter()
has the similar problem in the "err_pmu_ctx" error handling path:

CPU0					CPU1
perf_event_create_kernel_counter()
  // inc ctx refcnt
  find_get_context(task, event) (1)

  // inc pmu_ctx refcnt
  pmu_ctx = find_get_pmu_context()

  event->pmu_ctx = pmu_ctx
  ...
  goto err_pmu_ctx:
    // dec pmu_ctx refcnt
    put_pmu_ctx(pmu_ctx) (2)

    mutex_unlock(&ctx->mutex)
    // dec ctx refcnt
    put_ctx(ctx)
					perf_event_exit_task_context()
					  mutex_lock()
					  mutex_unlock()
					  // last refcnt put
					  put_ctx()
    free_event(event)
      if (event->pmu_ctx) // True
        put_pmu_ctx() (3)
          // will access freed pmu_ctx or ctx

      if (event->ctx) // False
        put_ctx()

(3) has UAF problem since the pmu_ctx maybe freed in (2), so also
should have "event->pmu_ctx = NULL;" in (2).