linux-kernel - Re: [PATCH v2] ftrace: Allow inline functions not inlined to be traced

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: <20230613114049.096a8844@gandalf.local.home>
Date:   Tue, 13 Jun 2023 11:40:49 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Mark Rutland <mark.rutland@....com>,
        Kees Cook <keescook@...omium.org>,
        Miguel Ojeda <ojeda@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Ingo Molnar <mingo@...nel.org>, Song Liu <song@...nel.org>,
        Josh Poimboeuf <jpoimboe@...nel.org>
Subject: Re: [PATCH v2] ftrace: Allow inline functions not inlined to be
 traced

On Mon, 12 Jun 2023 17:09:31 +0200
Thomas Gleixner <tglx@...utronix.de> wrote:

> >
> > Currently only x86 uses this.  
> 
> I assume this passes the objtool noinstr validation. If so, if would be
> helpful to document that.

I haven't run this through the full test suite. But I will check.

> >  /*
> >   * gcc provides both __inline__ and __inline as alternate spellings of
> > @@ -230,7 +240,7 @@ struct ftrace_likely_data {
> >   *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> >   * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
> >   */
> > -# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
> > +# define __no_kasan_or_inline __no_sanitize_address __notrace_inline __maybe_unused  
> 
> I'm not convinced that this is correct
> 
> >  # define __no_sanitize_or_inline __no_kasan_or_inline
> >  #else  
> 
> given that the !__SANITIZE_ADDRESS__ variant is:
> 
> >  # define __no_kasan_or_inline __always_inline  
> 
> which cannot be traced.
> 
> > @@ -247,7 +257,7 @@ struct ftrace_likely_data {
> >   * disable all instrumentation. See Kconfig.kcsan where this is mandatory.
> >   */
> >  # define __no_kcsan __no_sanitize_thread __disable_sanitizer_instrumentation
> > -# define __no_sanitize_or_inline __no_kcsan notrace __maybe_unused
> > +# define __no_sanitize_or_inline __no_kcsan __notrace_inline  __maybe_unused  
> 
> Ditto. 

I'll just keep the notrace on these.

> 
> >  #else
> >  # define __no_kcsan
> >  #endif
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index abe5c583bd59..b66ab0e6ce19 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -106,6 +106,13 @@ config HAVE_BUILDTIME_MCOUNT_SORT
> >           An architecture selects this if it sorts the mcount_loc section
> >  	 at build time.
> >  
> > +config ARCH_CAN_TRACE_INLINE
> > +       bool
> > +       help
> > +         It is safe for an architecture to trace any function marked  
> 
> Spaces instead of tab.

Bah, I noticed that my emacs is doing this on other configs I just added.
It adds spaces for the first entry, then tabs for the rest.

> 
> > +	 as inline (not __always_inline) that the compiler decides to  
> 
> and this one has a tab.
> 
> > +	 not inline.
> > +
> >  config BUILDTIME_MCOUNT_SORT
> >         bool
> >         default y  

Thanks for the review!

-- Steve