Re: NULL ptr deref in perf/filter_match

[Vegard Nossum]

On 4 August 2016 at 17:17, Vegard Nossum <vegard.nossum@xxxxxxxxx> wrote:
> On 4 August 2016 at 14:37, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>> On Fri, Jul 29, 2016 at 11:41:11PM +0200, Vegard Nossum wrote:
>>
>>> Digging a bit deeper into this, it seems the event itself is getting
>>> created by perf_event_open() and it gets added to the pmu_event_list
>>> through:
>>>
>>> perf_event_open()
>>>  - perf_event_alloc()
>>>     - account_event()
>>>        - account_pmu_sb_event()
>>>           - attach_sb_event()
>>>
> [...]
>>> So I guess the question is... should the event be attached (= put on
>>> the list) before ->ctx gets set? Or should the cgroup code check for a
>>> NULL ->ctx?
>>
>> Does this fix it? Ordering is a bit of a mess, adding the events to the
>> list _after_ they've been installed has the risk of missing things I
>> think, nor does that result in particularly nice code.
>>
>> Then again, this isn't pretty either.
>
> Patch seems to fix it here, thanks! Feel free to add:
>
> Tested-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>

BTW, this seems to show up slightly more frequently although I've seen
it a couple of times before without your patch too so it's probably
unrelated:

WARNING: CPU: 0 PID: 1244 at arch/x86/kernel/hw_breakpoint.c:121
arch_install_hw_breakpoint+0x284/0x2f0
Can't find any breakpoint slot
CPU: 0 PID: 1244 Comm: trinity-c0 Not tainted 4.7.0+ #73
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/20
14
ffffffff83a1a5a0 ffff880116b6f860 ffffffff81d75571 ffff880116b6f8d8
0000000000000000 ffff880116b6f8a8 ffffffff810fd28f ffff880115fb5a00
ffff880100000079 ffffed0022d6df17 0000000000000004 ffff88010e202a80
Call Trace:
[<ffffffff81d75571>] dump_stack+0x65/0x84
[<ffffffff810fd28f>] __warn+0x17f/0x1a0
[<ffffffff810fd342>] warn_slowpath_fmt+0x92/0xb0
[<ffffffff810fd2b0>] ? __warn+0x1a0/0x1a0
[<ffffffff8135b5ba>] ? perf_event_update_userpage+0x3ca/0x660
[<ffffffff81064144>] arch_install_hw_breakpoint+0x284/0x2f0
[<ffffffff81371e44>] hw_breakpoint_add+0xd4/0x100
[<ffffffff81354d37>] event_sched_in.isra.100+0x3a7/0xa40
[<ffffffff813554d3>] group_sched_in+0x103/0x3e0
[<ffffffff81066579>] ? sched_clock+0x9/0x10
[<ffffffff81355fbd>] ctx_sched_in+0x80d/0x16f0
[<ffffffff81356f00>] perf_event_sched_in+0x60/0x80
[<ffffffff81356f80>] ctx_resched+0x60/0xa0
[<ffffffff81357d67>] __perf_install_in_context+0x247/0x320
[<ffffffff81357b20>] ? __perf_event_enable+0xb60/0xb60
[<ffffffff81347fa0>] ? perf_duration_warn+0x40/0x40
[<ffffffff813480b5>] remote_function+0x115/0x1a0
[<ffffffff81347fa0>] ? perf_duration_warn+0x40/0x40
[<ffffffff8125d801>] generic_exec_single+0x191/0x290
[<ffffffff81347fa0>] ? perf_duration_warn+0x40/0x40
[<ffffffff8125db2e>] smp_call_function_single+0xde/0x350
[<ffffffff8125da50>] ? generic_smp_call_function_single_interrupt+0x10/0x10
[<ffffffff81341f80>] perf_install_in_context+0x200/0x370
[<ffffffff81341d80>] ? perf_remove_from_context+0xc0/0xc0
[<ffffffff81357b20>] ? __perf_event_enable+0xb60/0xb60
[<ffffffff81361482>] SYSC_perf_event_open+0xd62/0x1cf0
[<ffffffff81360720>] ? perf_event_set_output+0x400/0x400
[<ffffffff81dd4603>] ? __this_cpu_preempt_check+0x13/0x20
[<ffffffff81369e80>] ? perf_pmu_unregister+0x470/0x470
[<ffffffff81369e89>] SyS_perf_event_open+0x9/0x10
[<ffffffff81005391>] do_syscall_64+0x1a1/0x460
[<ffffffff813742fa>] ? __context_tracking_enter+0xaa/0x200
[<ffffffff8389746a>] entry_SYSCALL64_slow_path+0x25/0x25
---[ end trace 966c767fd836202d ]---


Vegard