Re: [PATCH v2 26/39] x86/cet/shstk: Introduce routines modifying shstk

[Edgecombe, Rick P]

On Wed, 2022-10-05 at 22:58 +0000, Andrew Cooper wrote:
> On 05/10/2022 23:47, Edgecombe, Rick P wrote:
> > On Wed, 2022-10-05 at 02:43 +0000, Andrew Cooper wrote:
> > > On 29/09/2022 23:29, Rick Edgecombe wrote:
> > > > diff --git a/arch/x86/include/asm/special_insns.h
> > > > b/arch/x86/include/asm/special_insns.h
> > > > index 35f709f619fb..f096f52bd059 100644
> > > > --- a/arch/x86/include/asm/special_insns.h
> > > > +++ b/arch/x86/include/asm/special_insns.h
> > > > @@ -223,6 +223,19 @@ static inline void clwb(volatile void
> > > > *__p)
> > > >                 : [pax] "a" (p));
> > > >    }
> > > >    
> > > > +#ifdef CONFIG_X86_SHADOW_STACK
> > > > +static inline int write_user_shstk_64(u64 __user *addr, u64
> > > > val)
> > > > +{
> > > > +     asm_volatile_goto("1: wrussq %[val], (%[addr])\n"
> > > > +                       _ASM_EXTABLE(1b, %l[fail])
> > > > +                       :: [addr] "r" (addr), [val] "r" (val)
> > > > +                       :: fail);
> > > 
> > > "1: wrssq %[val], %[addr]\n"
> > > _ASM_EXTABLE(1b, %l[fail])
> > > : [addr] "+m" (*addr)
> > > : [val] "r" (val)
> > > :: fail
> > > 
> > > Otherwise you've failed to tell the compiler that you wrote to
> > > *addr.
> > > 
> > > With that fixed, it's not volatile because there are no
> > > unexpressed
> > > side
> > > effects.
> > 
> > Ok, thanks!
> 
> On further consideration, it should be "=m" not "+m", which is even
> less
> constrained, and even easier for an enterprising optimiser to try and
> do
> something useful with.

AFAICT this won't work on all gccs. Asm goto's used to not support
having outputs. They are also implicitly volatile anyway. So I think
I'll have to leave it. But I can change the wrss one in the selftest to
"=m".