linux-kernel - Re: [RFC PATCH -tip 0/5] kprobes: Abolish jprobe APIs

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: <20171009103317.5701528f@lwn.net>
Date:   Mon, 9 Oct 2017 10:33:17 -0600
From:   Jonathan Corbet <corbet@....net>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Ingo Molnar <mingo@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        "H . Peter Anvin" <hpa@...or.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        Ian McDonald <ian.mcdonald@...di.co.nz>,
        Vlad Yasevich <vyasevich@...il.com>,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [RFC PATCH -tip 0/5] kprobes: Abolish jprobe APIs

On Mon, 9 Oct 2017 12:20:35 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> > > SAVE_REGS_IF_SUPPORTED - Similar to SAVE_REGS but the registering of a
> > >       ftrace_ops on an architecture that does not support passing of regs
> > >       will not fail with this flag set. But the callback must check if
> > >       regs is NULL or not to determine if the architecture supports it.
> > > 
> > > RECURSION_SAFE - By default, a wrapper is added around the callback to
> > >       make sure that recursion of the function does not occur. That is
> > >       if a function within the callback itself is also traced, ftrace    
> > 
> > s/within the/called by the/  
> 
> I put in "within" because it is usually a function that is nested
> within a function called by the callback. This bug has come up with
> "gotchas", where some function that the callback calls has a path to a
> function that was unexpectedly traced.
> 
> The issue hasn't been caused by a function being traced that was
> directly called by the callback. It is usually some deeper nested
> function.
> 
> I don't want to limit it to just checking functions that the callback
> calls. Thoughts on how to stress this?

"if a function that is called as a result of the callback's execution is
also traced" ?

> > > IPMODIFY - Requires SAVE_REGS set. If the callback is to "hijack" the
> > >       traced function (have another function called instead of the traced
> > >       function), it requires setting this flag. This is what live kernel
> > >       patches uses. Without this flag the pt_regs->ip can not be modified.
> > >       Note, only one ftrace_ops with IPMODIFY set may be registered to
> > >       any given function at a time.    
> > 
> > I assume this requires being able to get the regs too?    
> 
> Yes, this is why I stated "Requires SAVE_REGS" which would pass the
> regs to the callback. Should I rewrite that somehow?

No, just ship me another cup of coffee and that one should be OK.  Though
perhaps if you'd spelled out the flag completely I wouldn't have been so
dense :)

> > > If a glob is used to set the filter, to remove unwanted matches the
> > > ftrace_set_notrace() can also be used.
> > > 
> > >   int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
> > > 			 int len, int reset);
> > > 
> > > This takes the same parameters as ftrace_set_filter() but will add the
> > > functions it finds to not be traced. This doesn't remove them from the
> > > filter itself, but keeps them from being traced. If @reset is set,
> > > the filter is cleaded but the functions that match @buf will still not    
> > 
> > cleaded? :)  
> 
> Hmm, I'll have to be more descriptive.
> 
> >   
> > > be traced (the callback will not be called on those functions).    
> > 
> > So how do you clead the "notrace" list?  
> 
> With passing in reset non-zero. I'll add that.

My confusion remains here.  The text says that if reset is "set", then the
"notrace" list remains in place.  So a non-zero "reset" value will remove
previous notrace entries, along with the filter itself?  So if you wanted
to clear the notrace list entirely you would use buf="", reset=1?  It would
be good to be explicit there.

Thanks,

jon