Re: [PATCH v2] x86: Add -mharden-sls=[none|all|return|indirect-branch]

[Uros Bizjak <ubizjak@gmail.com>]

On Wed, Nov 17, 2021 at 4:35 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Add -mharden-sls= to mitigate against straight line speculation (SLS)
> for function return and indirect branch by adding an INT3 instruction
> after function return and indirect branch.
>
> gcc/
>
>         PR target/102952
>         * config/i386/i386-opts.h (harden_sls): New enum.
>         * config/i386/i386.c (output_indirect_thunk): Mitigate against
>         SLS for function return.
>         (ix86_output_function_return): Likewise.
>         (ix86_output_jmp_thunk_or_indirect): Mitigate against indirect
>         branch.
>         (ix86_output_indirect_jmp): Likewise.
>         (ix86_output_call_insn): Likewise.
>         * config/i386/i386.opt: Add -mharden-sls=.
>         * doc/invoke.texi: Document -mharden-sls=.
>
> gcc/testsuite/
>
>         PR target/102952
>         * gcc.target/i386/harden-sls-1.c: New test.
>         * gcc.target/i386/harden-sls-2.c: Likewise.
>         * gcc.target/i386/harden-sls-3.c: Likewise.
>         * gcc.target/i386/harden-sls-4.c: Likewise.
>         * gcc.target/i386/harden-sls-5.c: Likewise.
> ---
>  gcc/config/i386/i386-opts.h                  |  7 ++++++
>  gcc/config/i386/i386.c                       | 23 ++++++++++++++------
>  gcc/config/i386/i386.opt                     | 20 +++++++++++++++++
>  gcc/doc/invoke.texi                          | 10 ++++++++-
>  gcc/testsuite/gcc.target/i386/harden-sls-1.c | 14 ++++++++++++
>  gcc/testsuite/gcc.target/i386/harden-sls-2.c | 14 ++++++++++++
>  gcc/testsuite/gcc.target/i386/harden-sls-3.c | 14 ++++++++++++
>  gcc/testsuite/gcc.target/i386/harden-sls-4.c | 16 ++++++++++++++
>  gcc/testsuite/gcc.target/i386/harden-sls-5.c | 17 +++++++++++++++
>  9 files changed, 127 insertions(+), 8 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/harden-sls-1.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/harden-sls-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/harden-sls-3.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/harden-sls-4.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/harden-sls-5.c
>
> diff --git a/gcc/config/i386/i386-opts.h b/gcc/config/i386/i386-opts.h
> index 04e4ad608fb..171d3106d0a 100644
> --- a/gcc/config/i386/i386-opts.h
> +++ b/gcc/config/i386/i386-opts.h
> @@ -121,4 +121,11 @@ enum instrument_return {
>    instrument_return_nop5
>  };
>
> +enum harden_sls {
> +  harden_sls_none = 0,
> +  harden_sls_return = 1 << 0,
> +  harden_sls_indirect_branch = 1 << 1,
> +  harden_sls_all = harden_sls_return | harden_sls_indirect_branch
> +};
> +
>  #endif
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 73c4d5115bb..8bbf6ae9875 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -5914,6 +5914,8 @@ output_indirect_thunk (unsigned int regno)
>      }
>
>    fputs ("\tret\n", asm_out_file);
> +  if ((ix86_harden_sls & harden_sls_return))
> +    fputs ("\tint3\n", asm_out_file);
>  }
>
>  /* Output a funtion with a call and return thunk for indirect branch.
> @@ -15984,6 +15986,8 @@ ix86_output_jmp_thunk_or_indirect (const char *thunk_name, const int regno)
>        fprintf (asm_out_file, "\tjmp\t");
>        assemble_name (asm_out_file, thunk_name);
>        putc ('\n', asm_out_file);
> +      if ((ix86_harden_sls & harden_sls_indirect_branch))
> +       fputs ("\tint3\n", asm_out_file);
>      }
>    else
>      output_indirect_thunk (regno);
> @@ -16206,10 +16210,10 @@ ix86_output_indirect_jmp (rtx call_op)
>         gcc_unreachable ();
>
>        ix86_output_indirect_branch (call_op, "%0", true);
> -      return "";
>      }
>    else
> -    return "%!jmp\t%A0";
> +    output_asm_insn ("%!jmp\t%A0", &call_op);
> +  return (ix86_harden_sls & harden_sls_indirect_branch) ? "int3" : "";
>  }
>
>  /* Output return instrumentation for current function if needed.  */
> @@ -16277,10 +16281,10 @@ ix86_output_function_return (bool long_p)
>        return "";
>      }
>
> -  if (!long_p)
> -    return "%!ret";
> -
> -  return "rep%; ret";
> +  if ((ix86_harden_sls & harden_sls_return))
> +    long_p = false;

Is the above really needed? This will change "rep ret" to a "[notrack]
ret" when SLS hardening is in effect, with a conditional [notrack]
prefix, even when long ret was requested.

On a related note, "notrack ret" does not assemble for me, the
assembler reports:

notrack.s:1: Error: expecting indirect branch instruction after `notrack'

Can you please clarify the above change?

Uros.

> +  output_asm_insn (long_p ? "rep%; ret" : "%!ret", nullptr);
> +  return (ix86_harden_sls & harden_sls_return) ? "int3" : "";
>  }
>
>  /* Output indirect function return.  RET_OP is the function return
> @@ -16375,7 +16379,12 @@ ix86_output_call_insn (rtx_insn *insn, rtx call_op)
>        if (output_indirect_p && !direct_p)
>         ix86_output_indirect_branch (call_op, xasm, true);
>        else
> -       output_asm_insn (xasm, &call_op);
> +       {
> +         output_asm_insn (xasm, &call_op);
> +         if (!direct_p
> +             && (ix86_harden_sls & harden_sls_indirect_branch))
> +           return "int3";
> +       }
>        return "";
>      }
>
> diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
> index 46fad3cc038..8d499a5a4df 100644
> --- a/gcc/config/i386/i386.opt
> +++ b/gcc/config/i386/i386.opt
> @@ -1117,6 +1117,26 @@ mrecord-return
>  Target Var(ix86_flag_record_return) Init(0)
>  Generate a __return_loc section pointing to all return instrumentation code.
>
> +mharden-sls=
> +Target RejectNegative Joined Enum(harden_sls) Var(ix86_harden_sls) Init(harden_sls_none)
> +Generate code to mitigate against straight line speculation.
> +
> +Enum
> +Name(harden_sls) Type(enum harden_sls)
> +Known choices for mitigation against straight line speculation with -mharden-sls=:
> +
> +EnumValue
> +Enum(harden_sls) String(none) Value(harden_sls_none)
> +
> +EnumValue
> +Enum(harden_sls) String(all) Value(harden_sls_all)

Please put the above enum last.

> +
> +EnumValue
> +Enum(harden_sls) String(return) Value(harden_sls_return)
> +
> +EnumValue
> +Enum(harden_sls) String(indirect-branch) Value(harden_sls_indirect_branch)
> +
>  mavx512bf16
>  Target Mask(ISA2_AVX512BF16) Var(ix86_isa_flags2) Save
>  Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2, AVX, AVX2, AVX512F and
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index a22758d18ee..0265c160e02 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1427,7 +1427,7 @@ See RS/6000 and PowerPC Options.
>  -mstack-protector-guard-symbol=@var{symbol} @gol
>  -mgeneral-regs-only  -mcall-ms2sysv-xlogues -mrelax-cmpxchg-loop @gol
>  -mindirect-branch=@var{choice}  -mfunction-return=@var{choice} @gol
> --mindirect-branch-register -mneeded}
> +-mindirect-branch-register -mharden-sls=@var{choice} -mneeded}
>
>  @emph{x86 Windows Options}
>  @gccoptlist{-mconsole  -mcygwin  -mno-cygwin  -mdll @gol
> @@ -32401,6 +32401,14 @@ not be reachable in the large code model.
>  @opindex mindirect-branch-register
>  Force indirect call and jump via register.
>
> +@item -mharden-sls=@var{choice}
> +@opindex mharden-sls
> +Generate code to mitigate against straight line speculation (SLS) with
> +@var{choice}.  The default is @samp{none} which disables all SLS
> +hardening.  @samp{return} enables SLS hardening for function return.
> +@samp{indirect-branch} enables SLS hardening for indirect branch.
> +@samp{all} enables all SLS hardening.
> +
>  @end table
>
>  These @samp{-m} switches are supported in addition to the above
> diff --git a/gcc/testsuite/gcc.target/i386/harden-sls-1.c b/gcc/testsuite/gcc.target/i386/harden-sls-1.c
> new file mode 100644
> index 00000000000..6f70dc94a23
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/harden-sls-1.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mindirect-branch=thunk-extern -mharden-sls=all" } */
> +/* { dg-additional-options "-fno-pic" { target { ! *-*-darwin* } } } */
> +
> +extern void foo (void);
> +
> +void
> +bar (void)
> +{
> +  foo ();
> +}
> +
> +/* { dg-final { scan-assembler "jmp\[ \t\]+_?foo" } } */
> +/* { dg-final { scan-assembler-not {int3} } } */
> diff --git a/gcc/testsuite/gcc.target/i386/harden-sls-2.c b/gcc/testsuite/gcc.target/i386/harden-sls-2.c
> new file mode 100644
> index 00000000000..a7c59078d03
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/harden-sls-2.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mindirect-branch=thunk-extern -mharden-sls=all" } */
> +/* { dg-additional-options "-fno-pic" { target { ! *-*-darwin* } } } */
> +
> +extern void (*fptr) (void);
> +
> +void
> +foo (void)
> +{
> +  fptr ();
> +}
> +
> +/* { dg-final { scan-assembler "jmp\[ \t\]+_?__x86_indirect_thunk_(r|e)ax" } } */
> +/* { dg-final { scan-assembler-times "int3" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/harden-sls-3.c b/gcc/testsuite/gcc.target/i386/harden-sls-3.c
> new file mode 100644
> index 00000000000..1a6056b6d7b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/harden-sls-3.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mindirect-branch=thunk -mharden-sls=all" } */
> +/* { dg-additional-options "-fno-pic" { target { ! *-*-darwin* } } } */
> +
> +extern void (*fptr) (void);
> +
> +void
> +foo (void)
> +{
> +  fptr ();
> +}
> +
> +/* { dg-final { scan-assembler "jmp\[ \t\]+_?__x86_indirect_thunk_(r|e)ax" } } */
> +/* { dg-final { scan-assembler-times "int3" 2 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/harden-sls-4.c b/gcc/testsuite/gcc.target/i386/harden-sls-4.c
> new file mode 100644
> index 00000000000..f70dd1379d3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/harden-sls-4.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mindirect-branch=keep -mharden-sls=all" } */
> +/* { dg-additional-options "-fno-pic" { target { ! *-*-darwin* } } } */
> +
> +extern void (*fptr) (void);
> +
> +void
> +foo (void)
> +{
> +  fptr ();
> +}
> +
> +/* { dg-final { scan-assembler "jmp\[ \t\]+\\*_?fptr" { target { ! x32 } } } } */
> +/* { dg-final { scan-assembler "movl\[ \t\]+fptr\\(%rip\\), %eax" { target x32 } } } */
> +/* { dg-final { scan-assembler "jmp\[ \t\]+\\*%rax" { target x32 } } } */
> +/* { dg-final { scan-assembler-times "int3" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/i386/harden-sls-5.c b/gcc/testsuite/gcc.target/i386/harden-sls-5.c
> new file mode 100644
> index 00000000000..613c44c6f82
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/harden-sls-5.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mno-indirect-branch-register -mfunction-return=keep -mindirect-branch=thunk-extern -mharden-sls=return" } */
> +/* { dg-additional-options "-fno-pic" { target { ! *-*-darwin* } } } */
> +
> +typedef void (*dispatch_t)(long offset);
> +
> +dispatch_t dispatch;
> +
> +int
> +male_indirect_jump (long offset)
> +{
> +  dispatch(offset);
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times "ret" 1 } } */
> +/* { dg-final { scan-assembler-times "int3" 1 } } */
> --
> 2.33.1
>