linux-kernel - Re: [PATCH v2 4/4] x86/static_call: Add inline static call implementation for x86-64

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: <20181130222447.u5yxnsvxg7zqibie@treble>
Date:   Fri, 30 Nov 2018 16:24:47 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Lutomirski <luto@...capital.net>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Lutomirski <luto@...nel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>, mhiramat@...nel.org,
        jbaron@...mai.com, Jiri Kosina <jkosina@...e.cz>,
        David.Laight@...lab.com, bp@...en8.de, julia@...com,
        jeyu@...nel.org, Peter Anvin <hpa@...or.com>
Subject: Re: [PATCH v2 4/4] x86/static_call: Add inline static call
 implementation for x86-64

On Fri, Nov 30, 2018 at 11:16:34PM +0100, Rasmus Villemoes wrote:
> On 29/11/2018 20.22, Josh Poimboeuf wrote:
> > On Thu, Nov 29, 2018 at 02:16:48PM -0500, Steven Rostedt wrote:
> >>> and honestly, the way "static_call()" works now, can you guarantee
> >>> that the call-site doesn't end up doing that, and calling the
> >>> trampoline function for two different static calls from one indirect
> >>> call?
> >>>
> >>> See what I'm talking about? Saying "callers are wrapped in macros"
> >>> doesn't actually protect you from the compiler doing things like that.
> >>>
> >>> In contrast, if the call was wrapped in an inline asm, we'd *know* the
> >>> compiler couldn't turn a "call wrapper(%rip)" into anything else.
> >>
> >> But then we need to implement all numbers of parameters.
> > 
> > I actually have an old unfinished patch which (ab)used C macros to
> > detect the number of parameters and then setup the asm constraints
> > accordingly.  At the time, the goal was to optimize the BUG code.
> > 
> > I had wanted to avoid this kind of approach for static calls, because
> > "ugh", but now it's starting to look much more appealing.
> > 
> > Behold:
> > 
> > diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
> > index aa6b2023d8f8..d63e9240da77 100644
> > --- a/arch/x86/include/asm/bug.h
> > +++ b/arch/x86/include/asm/bug.h
> > @@ -32,10 +32,59 @@
> >  
> >  #ifdef CONFIG_DEBUG_BUGVERBOSE
> >  
> > -#define _BUG_FLAGS(ins, flags)						\
> > +#define __BUG_ARGS_0(ins, ...) \
> > +({\
> > +	asm volatile("1:\t" ins "\n"); \
> > +})
> > +#define __BUG_ARGS_1(ins, ...) \
> > +({\
> > +	asm volatile("1:\t" ins "\n" \
> > +		     : : "D" (ARG1(__VA_ARGS__))); \
> > +})
> > +#define __BUG_ARGS_2(ins, ...) \
> > +({\
> > +	asm volatile("1:\t" ins "\n" \
> > +		     : : "D" (ARG1(__VA_ARGS__)), \
> > +			 "S" (ARG2(__VA_ARGS__))); \
> > +})
> > +#define __BUG_ARGS_3(ins, ...) \
> > +({\
> > +	asm volatile("1:\t" ins "\n" \
> > +		     : : "D" (ARG1(__VA_ARGS__)), \
> > +			 "S" (ARG2(__VA_ARGS__)), \
> > +			 "d" (ARG3(__VA_ARGS__))); \
> > +})
> 
> wouldn't you need to tie all these to (unused) outputs as well as adding
> the remaining caller-saved registers to the clobber list? Maybe not for
> the WARN machinery(?), but at least for stuff that should look like a
> normal call to gcc? Then there's %rax which is either a clobber or an
> output, and if there's not to be a separate static_call_void(), one
> would need to do some __builtin_choose_expr(__same_type(void, f(...)), ...).

Yes, this is a crappy unfinished patch.  It should be ignored, and
perhaps even mercilessly mocked :-)

paravirt_types.h already does something similar today, and it's at least
more correct than this.

What I was trying to show was that you can use macros to count
arguments, like this:

  _BUG_ARGS(ins, NUM_ARGS(__VA_ARGS__), __VA_ARGS__);

which can make a macro look and act like a function call.  Though as
Steven pointed out, the concept falls apart after 6 arguments.

-- 
Josh