Re: [RFC][PATCH] x86: Add straight-line-speculation mitigation

[Peter Zijlstra]

On Thu, Oct 28, 2021 at 09:51:12AM -0700, Kees Cook wrote:
> On Thu, Oct 28, 2021 at 01:44:00PM +0200, Peter Zijlstra wrote:
> > Hi,
> > 
> > This little patch makes use of an upcomming GCC feature to mitigate
> > straight-line-speculation for x86:
> > 
> >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102952
> > 
> > It's built tested on x86_64-allyesconfig using GCC-12+patch and GCC-11.
> > It's also been boot tested on x86_64-defconfig+kvm_guest.config using
> > GCC-12+patch.
> > 
> > Enjoy!
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> 
> I'm all for such mitigations. In x86's case, it's small and easy. I do
> note, however, than arm64 maintainers weren't as impressed:
> https://lore.kernel.org/lkml/20210305095256.GA22536@willie-the-truck/

Yeah, I remembered some of that :-)

> What's the image size impact?

My x86_64-defconfig+kvm gives:

   text    data     bss     dec     hex filename
22940902        6964034 1323240 31228176        1dc8110 defconfig-build/vmlinux
22388944        6964034 1880296 31233274        1dc94fa defconfig-build/vmlinux

~538kb, which is quite impressive

> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -179,6 +179,10 @@ ifdef CONFIG_RETPOLINE
> >    endif
> >  endif
> >  
> > +ifdef CONFIG_SLS
> > +  KBUILD_CFLAGS += -mharden-sls=all
> > +endif
> > +
> >  KBUILD_LDFLAGS += -m elf_$(UTS_MACHINE)
> >  
> >  ifdef CONFIG_LTO_CLANG
> 
> Given the earlier patch for ARM, perhaps the Kconfig and Makefile chunks
> should be at the top level instead, making this feature easier to
> implement in other architectures?

Hence me having Cc'ed some ARM64 people

> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -3084,6 +3084,12 @@ static int validate_branch(struct objtoo
> >  		switch (insn->type) {
> >  
> >  		case INSN_RETURN:
> > +			if (next_insn && next_insn->type == INSN_TRAP) {
> > +				next_insn->ignore = true;
> > +			} else if (sls && !insn->retpoline_safe) {
> > +				WARN_FUNC("missing int3 after ret",
> > +					  insn->sec, insn->offset);
> > +			}
> >  			return validate_return(func, insn, &state);
> >  
> >  		case INSN_CALL:
> > @@ -3127,6 +3133,14 @@ static int validate_branch(struct objtoo
> >  			break;
> >  
> >  		case INSN_JUMP_DYNAMIC:
> > +			if (next_insn && next_insn->type == INSN_TRAP) {
> > +				next_insn->ignore = true;
> > +			} else if (sls && !insn->retpoline_safe) {
> > +				WARN_FUNC("missing int3 after indirect jump",
> > +					  insn->sec, insn->offset);
> > +			}
> > +
> > +			/* fallthrough */
> >  		case INSN_JUMP_DYNAMIC_CONDITIONAL:
> >  			if (is_sibling_call(insn)) {
> >  				ret = validate_sibling_call(file, insn, &state);
> 
> Oh very nice; I was going to ask "how can we make sure no bare 'ret's
> sneak back into .S files" and here it is. Excellent.

Yeah, there was no way I was going to do that without tooling ;-) I'd
not have found half of it.

> Random thought, not for this patch, but can objtool validate the int3
> linker padding too? (i.e. to double-check the behavior of
> 7705dc855797 ("x86/vmlinux: Use INT3 instead of NOP for linker fill bytes"))

Probably. there might be some weird corner cases between GCC alignment
nops and linker fillers I suppose.