Re: [RFC PATCH 5/5] selftest/x86: Add CET quick test

[Yu-cheng Yu <yu-cheng.yu@intel.com>]

On Fri, 2020-05-22 at 11:28 +0200, Peter Zijlstra wrote:
> On Thu, May 21, 2020 at 02:17:20PM -0700, Yu-cheng Yu wrote:
> 
> > +#pragma GCC push_options
> > +#pragma GCC optimize ("O0")
> > +void ibt_violation(void)
> > +{
> > +#ifdef __i386__
> > +	asm volatile("lea 1f, %eax");
> > +	asm volatile("jmp *%eax");
> > +#else
> > +	asm volatile("lea 1f, %rax");
> > +	asm volatile("jmp *%rax");
> > +#endif
> > +	asm volatile("1:");
> > +	result[test_id] = -1;
> > +	test_id++;
> > +	setcontext(&ucp);
> > +}
> > +
> > +void shstk_violation(void)
> > +{
> > +#ifdef __i386__
> > +	unsigned long x = 0;
> > +
> > +	((unsigned long *)&x)[2] = (unsigned long)stack_hacked;
> > +#else
> > +	unsigned long long x = 0;
> > +
> > +	((unsigned long long *)&x)[2] = (unsigned long)stack_hacked;
> > +#endif
> > +}
> > +#pragma GCC pop_options
> 
> This is absolutely atrocious.
> 
> The #pragma like Kees already said just need to go. Also, there's
> absolutely no clue what so ever what it attempts to achieve.
> 
> The __i386__ ifdeffery is horrible crap. Splitting an asm with #ifdef
> like that is also horrible crap.
> 
> This is not how you write code.
> 
> Get asm/asm.h into userspace and then write something like:
> 
> 
> void ibt_violation(void)
> {
> 	asm volatile("lea  1f, %" _ASM_AX "\n\t"
> 		     "jmp  *%" _ASM_AX "\n\t"
> 		     "1:\n\t" ::: "a");
> 
> 	WRITE_ONCE(result[test_id], -1);
> 	WRITE_ONCE(test_id, test_id+1);
> 
> 	setcontext(&ucp);
> }
> 
> void shstk_violation(void)
> {
> 	unsigned long x = 0;
> 
> 	WRITE_ONCE(x[2], stack_hacked);
> }

Thanks!  I will change it.

Yu-cheng