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

Peter Zijlstra
2021-10-28T17:11:39Z

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@infradead.org>
>
> 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.