linux-kernel - Re: WARNING in tracepoint_probe_register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180315122649.GB27628@krava>
Date:   Thu, 15 Mar 2018 13:26:49 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        rostedt <rostedt@...dmis.org>,
        syzbot <syzbot+9c0d616860575a73166a@...kaller.appspotmail.com>,
        Ingo Molnar <mingo@...hat.com>, acme <acme@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>
Subject: Re: WARNING in tracepoint_probe_register_prio (2)

On Thu, Mar 15, 2018 at 07:52:23AM -0400, Mathieu Desnoyers wrote:
> ----- On Mar 15, 2018, at 5:19 AM, Jiri Olsa jolsa@...hat.com wrote:
> 
> > On Thu, Mar 15, 2018 at 09:31:25AM +0100, Peter Zijlstra wrote:
> >> On Wed, Mar 14, 2018 at 05:37:46PM -0400, Steven Rostedt wrote:
> >> > On Tue, 13 Mar 2018 11:29:51 -0400 (EDT)
> >> > Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
> >> > 
> >> > > Here is a WARN_ON() splat in tracepoint.c, which I suspect is caused
> >> > > by perf trying to register the same probe twice to the tracepoint API.
> >> > > We got another splat on unregister too, which I will forward in a
> >> > > separate email.
> >> > > 
> >> > > Thoughts ?
> >> > 
> >> > Yes, it looks like it's perf not accounting for registered events
> >> > properly.
> >> > 
> >> > Peter?
> >> 
> >> I've not yet managed to reproduce, but if you look at the provided
> >> repro.c file, you'll see it opens two _different_ events.
> > 
> > from the log it looks like they inject the slab error,
> > and the allocation fails.. looks like we need to change
> > the WARN to skip ENOMEM.. something like below?
> 
> Oh, I missed this important point. Then we should only
> warn if !-ENOMEM for both tracepoint_add_func and tracepoint_remove_func,
> because each performs memory allocation under the hood.
> Like the following:
> 
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -207,7 +207,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
>                         lockdep_is_held(&tracepoints_mutex));
>         old = func_add(&tp_funcs, func, prio);
>         if (IS_ERR(old)) {
> -               WARN_ON_ONCE(1);
> +               WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
>                 return PTR_ERR(old);
>         }
>  
> @@ -239,7 +239,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
>                         lockdep_is_held(&tracepoints_mutex));
>         old = func_remove(&tp_funcs, func);

right, overlooked this one ;-) looks good

jirka

>         if (IS_ERR(old)) {
> -               WARN_ON_ONCE(1);
> +               WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
>                 return PTR_ERR(old);
>         }
>  
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com