Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI

[Vegard Nossum]

On 18 August 2016 at 12:52, tip-bot for David Carrillo-Cisneros
<tipbot@xxxxxxxxx> wrote:
> Commit-ID:  71e7bc2bab77e64882c031c2af943c3256c1adb0
> Gitweb:     http://git.kernel.org/tip/71e7bc2bab77e64882c031c2af943c3256c1adb0
> Author:     David Carrillo-Cisneros <davidcc@xxxxxxxxxx>
> AuthorDate: Wed, 17 Aug 2016 13:55:04 -0700
> Committer:  Ingo Molnar <mingo@xxxxxxxxxx>
> CommitDate: Thu, 18 Aug 2016 10:35:52 +0200
>
> perf/core: Check return value of the perf_event_read() IPI
>
> The call to smp_call_function_single in perf_event_read() may fail if
> an invalid or not online CPU index is passed. Warn user if such bug is
> present and return error.
>
> Signed-off-by: David Carrillo-Cisneros <davidcc@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Kan Liang <kan.liang@xxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Paul Turner <pjt@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Stephane Eranian <eranian@xxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Vegard Nossum <vegard.nossum@xxxxxxxxx>
> Cc: Vince Weaver <vincent.weaver@xxxxxxxxx>
> Link: http://lkml.kernel.org/r/1471467307-61171-2-git-send-email-davidcc@xxxxxxxxxx
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
>  kernel/events/core.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a5fc5c8..5650f53 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3549,9 +3549,10 @@ static int perf_event_read(struct perf_event *event, bool group)
>                         .group = group,
>                         .ret = 0,
>                 };
> -               smp_call_function_single(event->oncpu,
> -                                        __perf_event_read, &data, 1);
> -               ret = data.ret;
> +               ret = smp_call_function_single(event->oncpu, __perf_event_read, &data, 1);
> +               /* The event must have been read from an online CPU: */
> +               WARN_ON_ONCE(ret);
> +               ret = ret ? : data.ret;
>         } else if (event->state == PERF_EVENT_STATE_INACTIVE) {
>                 struct perf_event_context *ctx = event->ctx;
>                 unsigned long flags;

Hi,

I'm running into this new warning now:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 23442 at kernel/events/core.c:3554
perf_event_read+0x472/0x590
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 23442 Comm: syz-executor Not tainted 4.8.0-rc2+ #145
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
0000000000000000 ffff88010d1bf808 ffffffff81f9f201 ffffffff83e72d60
ffff88010d1bf8e0 ffffffff83ecbd00 ffffffff847070c0 ffff88010d1bf8d0
ffffffff8150538a 0000000041b58ab3 ffffffff844dc5dd ffffffff815051a7
Call Trace:
[<ffffffff81f9f201>] dump_stack+0x83/0xb2
[<ffffffff8150538a>] panic+0x1e3/0x3a1
[<ffffffff815051a7>] ? set_ti_thread_flag+0x1e/0x1e
[<ffffffff81505a58>] ? rt_mutex_top_waiter.part.10+0x6/0x6
[<ffffffff813dd140>] ? generic_exec_single+0xf0/0x2d0
[<ffffffff814d86a2>] ? perf_event_read+0x472/0x590
[<ffffffff8126df4f>] __warn+0x1bf/0x1e0
[<ffffffff8126e13c>] warn_slowpath_null+0x2c/0x40
[<ffffffff814d86a2>] perf_event_read+0x472/0x590
[<ffffffff814d8230>] ? perf_event_set_addr_filter+0xad0/0xad0
[<ffffffff814d74fd>] ? perf_event_ctx_lock_nested+0xdd/0x1e0
[<ffffffff814e8aa4>] perf_event_read_value+0x84/0x520
[<ffffffff814d75bc>] ? perf_event_ctx_lock_nested+0x19c/0x1e0
[<ffffffff814d745b>] ? perf_event_ctx_lock_nested+0x3b/0x1e0
[<ffffffff814e9383>] perf_read+0x443/0x8d0
[<ffffffff814e8f40>] ? perf_event_read_value+0x520/0x520
[<ffffffff81e5ae62>] ? common_file_perm+0x2e2/0x380
[<ffffffff816709a2>] do_loop_readv_writev+0x152/0x200
[<ffffffff81d935f6>] ? security_file_permission+0x86/0x1e0
[<ffffffff814e8f40>] ? perf_event_read_value+0x520/0x520
[<ffffffff814e8f40>] ? perf_event_read_value+0x520/0x520
[<ffffffff81673174>] do_readv_writev+0x614/0x700
[<ffffffff81672b60>] ? rw_verify_area+0x2b0/0x2b0
[<ffffffff814d7342>] ? put_ctx+0x22/0x100
[<ffffffff816d16f1>] ? __fget+0x1c1/0x270
[<ffffffff816d1577>] ? __fget+0x47/0x270
[<ffffffff81676d5b>] vfs_readv+0x8b/0xc0
[<ffffffff81676e6e>] do_readv+0xde/0x230
[<ffffffff81676d90>] ? vfs_readv+0xc0/0xc0
[<ffffffff81002b60>] ? exit_to_usermode_loop+0x190/0x190
[<ffffffff82001b07>] ? check_preemption_disabled+0x37/0x1e0
[<ffffffff81677617>] SyS_readv+0x27/0x30
[<ffffffff816775f0>] ? do_pwritev+0x1a0/0x1a0
[<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0
[<ffffffff83c3286a>] entry_SYSCALL64_slow_path+0x25/0x25

I don't think WARN() is the right interface for signalling errors to
userspace programs?


Vegard