linux-kernel - Re: [PATCH v2] tracing: Fix event probe removal from dynamic events

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211012200856.6e5b8f28@gandalf.local.home>
Date:   Tue, 12 Oct 2021 20:08:56 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Tom Zanussi <zanussi@...nel.org>,
        Tzvetomir Stoyanov <tz.stoyanov@...il.com>,
        Yordan Karadzhov <y.karadz@...il.com>
Subject: Re: [PATCH v2] tracing: Fix event probe removal from dynamic events

On Wed, 13 Oct 2021 07:42:26 +0900
Masami Hiramatsu <mhiramat@...nel.org> wrote:

> On Tue, 12 Oct 2021 12:03:10 -0400
> Steven Rostedt <rostedt@...dmis.org> wrote:
> 
> > On Tue, 12 Oct 2021 23:31:07 +0900
> > Masami Hiramatsu <mhiramat@...nel.org> wrote:
> >   
> > > Hmm, this seems something wrong. Via dynamic_events interface, all the
> > > events must be parsed equaly. If you have to pass the attached "system/event"
> > > that's something wrong. The dynamic_events interface will accept 
> > > 
> > > -:[GROUP/]EVENT [optional arguments]
> > > 
> > > Or
> > > 
> > > !e:[GROUP/]EVENT [optional arguments]
> > > 
> > > What did you expect other that these syntax?  
> > 
> > But there are non "optional arguments".
> > 
> > To create the event probe, we need to send:
> > 
> >   e:[GROUP/]EVENT system/event [optional arguments]
> > 
> > Where the "system/event" is what we attach to. Similar to adding a function
> > or address to kprobes. Do you not need to add that for deleting a kprobe?  
> 
> No, since if the GROUP name is given, we can identify the event.
> 
> And sorry. I misunderstood your patch, simply I mixed the group/event is
> the name of group/event or the attached group/event.

The GROUP/EVENT is the name of the created eprobe, my "system/event" is the
name of the probe being attached.

> 
> Actually, the dynamic_events delete command is something like wildcard
> unless you specify the options.

OK, so we need to update this a little bit more. But currently, it fails if
you do:

 # echo 'e:hrstate timer/hrtimer_cancel state=+0x38($hrtimer):u8' >> dynamic_events
 # cat dynamic_events
eprobes/hrstate timer.hrtimer_cancel state=+0x38($hrtimer):u8
 # echo '-:eprobes/hrstate timer.hrtimer_cancel state=+0x38($hrtimer):u8' >> dynamic_events

It will error out with "-EBUSY".

It would make sense if we echo in exactly what is in dynamic_events with
"-:" in front of it it, that it will remove it. But currently, it expects:

 # echo '-:eprobes/hrstate state=+0x38($hrtimer):u8' >> dynamic_events

Which is completely wrong.


> 
> > 
> > That is, if I create a kprobe with:
> > 
> >   p:myprobe schedule > dynamic_events
> > 
> > To remove it, don't we need to have:
> > 
> >   -:myprobe schedule >> dynamic_events  
> 
> Yes, it is possible. But you can also do
> 
>  -:kprobes/myprobe >> dynamic_events 
> 
> So, the "schedule" trace point is optional.
> 
> Anyway, let me comment on your patch again.

OK, we can make it optional, but we need to fix it so that it also works.

-- Steve