linux-kernel - Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder

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: <20170810144232.j4wzf5rrl7vz2uu5@treble>
Date:   Thu, 10 Aug 2017 09:42:32 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Juergen Gross <jgross@...e.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        "Levin, Alexander (Sasha Levin)" <alexander.levin@...izon.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "live-patching@...r.kernel.org" <live-patching@...r.kernel.org>,
        Jiri Slaby <jslaby@...e.cz>, Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Mike Galbraith <efault@....de>
Subject: Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder

On Thu, Aug 10, 2017 at 09:09:03AM -0500, Josh Poimboeuf wrote:
> static inline notrace unsigned long arch_local_save_flags(void)
> {
> 	return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl,
> 			    "pushfq; popq %rax", CPU_FEATURE_NATIVE,
> 			    "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN,
> 			    "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP,
> 			    "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST);
> } 

Just a few clarifications on this idea:

It would probably be better to have the PVOP macros do the function name
translation, so maybe it would be something like this instead:

 	return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl,
 			    "pushfq; popq %rax", CPU_FEATURE_NATIVE,
 			    xen_save_fl, CPU_FEATURE_XEN,
 			    vsmp_save_fl, CPU_FEATURE_VSMP,
 			    lguest_save_fl, CPU_FEATURE_LGUEST);

One issue is that it would fail to link if CONFIG_XEN or
CONFIG_LGUEST_GUEST isn't set.  However I've seen some crazy macro magic
which lets you detect the number of variable arguments.  So if we got
that to work, it could be:

 	return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl,
 			    "pushfq; popq %rax", CPU_FEATURE_NATIVE,
#ifdef CONFIG_XEN
 			    xen_save_fl, CPU_FEATURE_XEN,
#endif
 			    vsmp_save_fl, CPU_FEATURE_VSMP,
#ifdef CONFIG_LGUEST_GUEST
 			    lguest_save_fl, CPU_FEATURE_LGUEST,
#endif
			    );

And then maybe the PVOP_CALL macros could detect the number of arguments
and call the corresponding version of ALTERNATIVE_X.  Macro fun :-)

> Which would eventually translate to something like:
> 
> asm volatile(ALTERNATIVE_4("call *pv_irq_ops.save_fl",
> 			   "pushfq; popq %rax", CPU_FEATURE_NATIVE,
> 			   "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN,
> 			   "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP,
> 			   "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST
> 			   : ... pvop clobber stuff ... );
> 
> where ALTERNATIVE_4 is a logical extension of ALTERNATIVE_2 and
> CPU_FEATURE_NATIVE would always be set.
> 
> It might need some more macro magic, but if it worked I think it would
> be a lot clearer than the current voodoo.
> 
> Thoughts?

-- 
Josh