linux-kernel - Re: [RFC PATCH 1/3] static_call: Add static call infrastructure

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:   Sun, 11 Nov 2018 21:07:22 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jason Baron <jbaron@...mai.com>, Jiri Kosina <jkosina@...e.cz>,
        David Laight <David.Laight@...lab.com>,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [RFC PATCH 1/3] static_call: Add static call infrastructure

On Sat, Nov 10, 2018 at 08:09:17AM -0500, Steven Rostedt wrote:
> On Sat, 10 Nov 2018 12:58:08 +0100
> Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
> 
> 
> > > The complaint is on:
> > >
> > >         DEFINE_STATIC_CALL(__tp_func_##name, __tracepoint_iter_##name);
> > >
> > > And the previous definition is on:
> > >
> > >         DECLARE_STATIC_CALL(__tp_func_##name, __tracepoint_iter_##name); \
> > >  
> > 
> > Does the DECLARE really need the __ADDRESSABLE? Its purpose is to
> > ensure that symbols with static linkage are not optimized away, but
> > since the reference is from a header file, the symbol should have
> > external linkage anyway.

Yes, DECLARE needs the __ADDRESSABLE.  In the case where DECLARE
is used, but DEFINE is not, GCC strips the symbol.

> I applied the following change and it compiled fine:
> 
> diff --git a/include/linux/static_call.h b/include/linux/static_call.h
> index 90b580b95303..5f8a0f0e77be 100644
> --- a/include/linux/static_call.h
> +++ b/include/linux/static_call.h
> @@ -108,8 +108,6 @@ extern void arch_static_call_poison_tramp(unsigned long insn);
>  #define DECLARE_STATIC_CALL(key, func)					\
>  	extern struct static_call_key key;				\
>  	extern typeof(func) STATIC_CALL_TRAMP(key);			\
> -	/* Preserve the ELF symbol so objtool can access it: */		\
> -	__ADDRESSABLE(key)
>  
>  #define DEFINE_STATIC_CALL(key, _func)					\
>  	DECLARE_STATIC_CALL(key, _func);				\
> @@ -117,7 +115,9 @@ extern void arch_static_call_poison_tramp(unsigned long insn);
>  		.func = _func,						\
>  		.site_mods = LIST_HEAD_INIT(key.site_mods),		\
>  	};								\
> -	ARCH_STATIC_CALL_TEMPORARY_TRAMP(key)
> +	ARCH_STATIC_CALL_TEMPORARY_TRAMP(key);				\
> +	/* Preserve the ELF symbol so objtool can access it: */		\
> +	__ADDRESSABLE(key)
>  
>  #define static_call(key, args...) STATIC_CALL_TRAMP(key)(args)

Adding __ADDRESSABLE to the DEFINE macro doesn't do any good.  By
definition, the key is defined in the .o file, so the symbol already
exists.

The issue you're seeing is really an issue with the __ADDRESSABLE macro
not creating unique symbol names.  This should fix it:

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 06396c1cf127..4bb73fd918b5 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -282,7 +282,7 @@ unsigned long read_word_at_a_time(const void *addr)
  */
 #define __ADDRESSABLE(sym) \
 	static void * __section(".discard.addressable") __used \
-		__PASTE(__addressable_##sym, __LINE__) = (void *)&sym;
+		__UNIQUE_ID(__addressable_##sym) = (void *)&sym;
 
 /**
  * offset_to_ptr - convert a relative memory offset to an absolute pointer