linux-kernel - Re: [PATCH 1/4] ftrace: pass fregs to arch_ftrace_set_direct

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]

Date:   Mon, 24 Oct 2022 18:04:49 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, catalin.marinas@....com,
        linux-arm-kernel@...ts.infradead.org, mhiramat@...nel.org,
        revest@...omium.org, will@...nel.org
Subject: Re: [PATCH 1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller()

On Mon, Oct 24, 2022 at 10:48:45AM -0400, Steven Rostedt wrote:
> On Mon, 24 Oct 2022 15:08:43 +0100
> Mark Rutland <mark.rutland@....com> wrote:
> 
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -429,6 +429,7 @@ static inline int modify_ftrace_direct_multi_nolock(struct ftrace_ops *ops, unsi
> >  }
> >  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
> >  
> > +#ifdef CONFIG_FUNCTION_TRACER
> 
> Instead of adding the above preprocessor check, the below chunk should be
> moved into the CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS block above.

Sure; but note that doing that naively means 'struct ftrace_regs' won't always
be declared (e.g. if !CONFIG_FUNCTION_TRACER), and will result in warnings, e.g.

|   CC      arch/x86/kernel/asm-offsets.s
| In file included from ./include/linux/kvm_host.h:32,
|                  from arch/x86/kernel/../kvm/vmx/vmx.h:5,
|                  from arch/x86/kernel/asm-offsets.c:22:
| ./include/linux/ftrace.h:444:57: error: ‘struct ftrace_regs’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
|   444 | static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs,
|       |                                                         ^~~~~~~~~~~
| cc1: all warnings being treated as errors
| make[1]: *** [scripts/Makefile.build:118: arch/x86/kernel/asm-offsets.s] Error 1
| make: *** [Makefile:1270: prepare0] Error 2

... so I'll either need to add some ifdeffery, for CONFIG_FUNCTION_TRACER, or I
can hoist the declaration of 'struct ftrace_regs' to not depend on
CONFIG_FUNCTION_TRACER.

I guess the latter is preferable, e.g.

| diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
| index 2b34fec40a39..f201fcbfffb0 100644
| --- a/include/linux/ftrace.h
| +++ b/include/linux/ftrace.h
| @@ -37,9 +37,10 @@ extern void ftrace_boot_snapshot(void);
|  static inline void ftrace_boot_snapshot(void) { }
|  #endif
|  
| -#ifdef CONFIG_FUNCTION_TRACER
|  struct ftrace_ops;
|  struct ftrace_regs;
| +
| +#ifdef CONFIG_FUNCTION_TRACER
|  /*
|   * If the arch's mcount caller does not support all of ftrace's
|   * features, then it must call an indirect function that

... so I've done that locally for now.

Thanks,
Mark.