Re: [PATCH 2/4] rs6000: Emit ROP-protect instructions in prologue and epilogue

[will schmidt <will_schmidt@vnet.ibm.com>]

On Sun, 2021-04-25 at 20:50 -0500, Bill Schmidt via Gcc-patches wrote:
> Insert the hashst and hashchk instructions when -mrop-protect has been
> selected.  The encrypted save slot for ROP mitigation is placed
> between the parameter save area and the alloca space (if any;
> otherwise the local variable space).
> 
> Note that ROP-mitigation instructions are currently only provided for
> the ELFv2 ABI.
> 
> 2021-03-25  Bill Schmidt  <wschmidt@linux.ibm.com>
> 
> gcc/
> 	* config/rs6000/rs6000-internal.h (rs6000_stack): Add
> 	rop_check_save_offset and rop_check_size.
> 	* config/rs6000/rs6000-logue.c (rs6000_stack_info): Compute
> 	rop_check_size and rop_check_save_offset.
> 	(debug_stack_info): Dump rop_save_offset and rop_check_size.
> 	(rs6000_emit_prologue): Assert if WORLD_SAVE used with
> 	-mrop-protect; emit hashst[p] in prologue; emit hashchk[p] in
> 	epilogue.
> 	* config/rs6000/rs6000.md (unspec): Add UNSPEC_HASHST[P] and
> 	UNSPEC_HASHCHK[P].
> 	(hashst): New define_insn.
> 	(hashstp): Likewise.
> 	(hashchk): Likewise.
> 	(hashchkp): Likewise.

ok

> ---
>  gcc/config/rs6000/rs6000-internal.h |  2 +
>  gcc/config/rs6000/rs6000-logue.c    | 86 +++++++++++++++++++++++++----
>  gcc/config/rs6000/rs6000.md         | 39 +++++++++++++
>  3 files changed, 116 insertions(+), 11 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-internal.h b/gcc/config/rs6000/rs6000-internal.h
> index 428a7861a98..8fc77ba6138 100644
> --- a/gcc/config/rs6000/rs6000-internal.h
> +++ b/gcc/config/rs6000/rs6000-internal.h
> @@ -39,6 +39,7 @@ typedef struct rs6000_stack {
>    int gp_save_offset;		/* offset to save GP regs from initial SP */
>    int fp_save_offset;		/* offset to save FP regs from initial SP */
>    int altivec_save_offset;	/* offset to save AltiVec regs from initial SP */
> +  int rop_check_save_offset;	/* offset to save ROP check from initial SP */
>    int lr_save_offset;		/* offset to save LR from initial SP */
>    int cr_save_offset;		/* offset to save CR from initial SP */
>    int vrsave_save_offset;	/* offset to save VRSAVE from initial SP */
> @@ -53,6 +54,7 @@ typedef struct rs6000_stack {
>    int gp_size;			/* size of saved GP registers */
>    int fp_size;			/* size of saved FP registers */
>    int altivec_size;		/* size of saved AltiVec registers */
> +  int rop_check_size;		/* size of ROP check slot */
>    int cr_size;			/* size to hold CR if not in fixed area */
>    int vrsave_size;		/* size to hold VRSAVE */
>    int altivec_padding_size;	/* size of altivec alignment padding */

ok

> diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c
> index b0ac183ceff..10cf7a2de93 100644
> --- a/gcc/config/rs6000/rs6000-logue.c
> +++ b/gcc/config/rs6000/rs6000-logue.c
> @@ -595,19 +595,21 @@ rs6000_savres_strategy (rs6000_stack_t *info,
>  		+---------------------------------------+
>  		| Parameter save area (+padding*) (P)	|  32
>  		+---------------------------------------+
> -		| Alloca space (A)			|  32+P
> +		| Optional ROP check slot (R)		|  32+P
>  		+---------------------------------------+
> -		| Local variable space (L)		|  32+P+A
> +		| Alloca space (A)			|  32+P+R
>  		+---------------------------------------+
> -		| Save area for AltiVec registers (W)	|  32+P+A+L
> +		| Local variable space (L)		|  32+P+R+A
>  		+---------------------------------------+
> -		| AltiVec alignment padding (Y)		|  32+P+A+L+W
> +		| Save area for AltiVec registers (W)	|  32+P+R+A+L
>  		+---------------------------------------+
> -		| Save area for GP registers (G)	|  32+P+A+L+W+Y
> +		| AltiVec alignment padding (Y)		|  32+P+R+A+L+W
>  		+---------------------------------------+
> -		| Save area for FP registers (F)	|  32+P+A+L+W+Y+G
> +		| Save area for GP registers (G)	|  32+P+R+A+L+W+Y
>  		+---------------------------------------+
> -	old SP->| back chain to caller's caller		|  32+P+A+L+W+Y+G+F
> +		| Save area for FP registers (F)	|  32+P+R+A+L+W+Y+G
> +		+---------------------------------------+
> +	old SP->| back chain to caller's caller		|  32+P+R+A+L+W+Y+G+F
>  		+---------------------------------------+
> 
>       * If the alloca area is present, the parameter save area is

ok

> @@ -717,6 +719,19 @@ rs6000_stack_info (void)
>    /* Does this function call anything (apart from sibling calls)?  */
>    info->calls_p = (!crtl->is_leaf || cfun->machine->ra_needs_full_frame);
> 
> +  if (TARGET_POWER10 && info->calls_p
> +      && DEFAULT_ABI == ABI_ELFv2 && rs6000_rop_protect)
> +    info->rop_check_size = 8;
> +  else if (rs6000_rop_protect && DEFAULT_ABI != ABI_ELFv2)
> +    {
> +      /* We can't check this in rs6000_option_override_internal since
> +	 DEFAULT_ABI isn't established yet.  */
> +      error ("%qs requires the ELFv2 ABI", "-mrop-protect");
> +      info->rop_check_size = 0;
ok
> +    }
> +  else
> +    info->rop_check_size = 0;
> +
ok

>    /* Determine if we need to save the condition code registers.  */
>    if (save_reg_p (CR2_REGNO)
>        || save_reg_p (CR3_REGNO)
> @@ -806,8 +821,14 @@ rs6000_stack_info (void)
>  	  gcc_assert (info->altivec_size == 0
>  		      || info->altivec_save_offset % 16 == 0);
> 
> -	  /* Adjust for AltiVec case.  */
> -	  info->ehrd_offset = info->altivec_save_offset - ehrd_size;
> +	  if (info->rop_check_size != 0)
> +	    {
> +	      info->rop_check_save_offset
> +		= info->altivec_save_offset - info->rop_check_size;
> +	      info->ehrd_offset = info->rop_check_save_offset - ehrd_size;
> +	    }
> +	  else
> +	    info->ehrd_offset = info->altivec_save_offset - ehrd_size;
ok

>  	}
>        else
>  	info->ehrd_offset = info->gp_save_offset - ehrd_size;
> @@ -849,6 +870,7 @@ rs6000_stack_info (void)
>  				  + info->gp_size
>  				  + info->altivec_size
>  				  + info->altivec_padding_size
> +				  + info->rop_check_size
>  				  + ehrd_size
>  				  + ehcr_size
>  				  + info->cr_size
> @@ -987,6 +1009,10 @@ debug_stack_info (rs6000_stack_t *info)
>      fprintf (stderr, "\tvrsave_save_offset  = %5d\n",
>  	     info->vrsave_save_offset);
> 
> +  if (info->rop_check_size)
> +    fprintf (stderr, "\trop_check_save_offset = %5d\n",
> +	     info->rop_check_save_offset);
> +

ok

>    if (info->lr_save_p)
>      fprintf (stderr, "\tlr_save_offset      = %5d\n", info->lr_save_offset);
> 
> @@ -1026,6 +1052,9 @@ debug_stack_info (rs6000_stack_t *info)
>      fprintf (stderr, "\taltivec_padding_size= %5d\n",
>  	     info->altivec_padding_size);
> 
> +  if (info->rop_check_size)
> +    fprintf (stderr, "\trop_check_size      = %5d\n", info->rop_check_size);
> +
>    if (info->cr_size)
>      fprintf (stderr, "\tcr_size             = %5d\n", info->cr_size);
> 
> @@ -3044,7 +3073,6 @@ rs6000_emit_prologue (void)
>  	cfun->machine->r2_setup_needed = true;
>      }
> 
> -
>    if (flag_stack_usage_info)
>      current_function_static_stack_size = info->total_size;
> 
> @@ -3105,7 +3133,8 @@ rs6000_emit_prologue (void)
>  		  && (!crtl->calls_eh_return
>  		      || info->ehrd_offset == -432)
>  		  && info->vrsave_save_offset == -224
> -		  && info->altivec_save_offset == -416);
> +		  && info->altivec_save_offset == -416
> +		  && !rs6000_rop_protect);
> 

ok


>        treg = gen_rtx_REG (SImode, 11);
>        emit_move_insn (treg, GEN_INT (-info->total_size));
> @@ -3252,6 +3281,24 @@ rs6000_emit_prologue (void)
>  	}
>      }
> 
> +  /* The ROP hash store must occur before a stack frame is created.  */
> +  /* NOTE: The hashst isn't needed if we're going to do a sibcall,
> +     but there's no way to know that here.  Harmless except for
> +     performance, of course.  */
> +  if (TARGET_POWER10 && rs6000_rop_protect && info->rop_check_size != 0)
> +    {
> +      gcc_assert (DEFAULT_ABI == ABI_ELFv2);
> +      rtx stack_ptr = gen_rtx_REG (Pmode, STACK_POINTER_REGNUM);
> +      rtx addr = gen_rtx_PLUS (Pmode, stack_ptr,
> +			       GEN_INT (info->rop_check_save_offset));
> +      rtx mem = gen_rtx_MEM (Pmode, addr);
> +      rtx reg0 = gen_rtx_REG (Pmode, 0);
> +      if (rs6000_privileged)
> +	emit_insn (gen_hashstp (mem, reg0));
> +      else
> +	emit_insn (gen_hashst (mem, reg0));
> +    }
> +

ok


>    /* If we need to save CR, put it into r12 or r11.  Choose r12 except when
>       r12 will be needed by out-of-line gpr save.  */
>    cr_save_regno = ((DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
> @@ -4980,6 +5027,23 @@ rs6000_emit_epilogue (enum epilogue_type epilogue_type)
>        emit_insn (gen_add3_insn (sp_reg_rtx, sp_reg_rtx, sa));
>      }
> 
> +  /* The ROP hash check must occur after the stack pointer is restored,
> +     and is not performed for a sibcall.  */
> +  if (TARGET_POWER10 && rs6000_rop_protect && info->rop_check_size != 0
> +      && epilogue_type != EPILOGUE_TYPE_SIBCALL)
> +    {
> +      gcc_assert (DEFAULT_ABI == ABI_ELFv2);
> +      rtx stack_ptr = gen_rtx_REG (Pmode, STACK_POINTER_REGNUM);
> +      rtx addr = gen_rtx_PLUS (Pmode, stack_ptr,
> +			       GEN_INT (info->rop_check_save_offset));
> +      rtx mem = gen_rtx_MEM (Pmode, addr);
> +      rtx reg0 = gen_rtx_REG (Pmode, 0);
> +      if (rs6000_privileged)
> +	emit_insn (gen_hashchkp (reg0, mem));
> +      else
> +	emit_insn (gen_hashchk (reg0, mem));
> +    }
> +

ok

>    if (epilogue_type != EPILOGUE_TYPE_SIBCALL && restoring_FPRs_inline)
>      {
>        if (cfa_restores)
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index c8cdc42533c..eec2e7532e1 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -154,6 +154,10 @@ (define_c_enum "unspec"
>     UNSPEC_CNTTZDM
>     UNSPEC_PDEPD
>     UNSPEC_PEXTD
> +   UNSPEC_HASHST
> +   UNSPEC_HASHSTP
> +   UNSPEC_HASHCHK
> +   UNSPEC_HASHCHKP
>    ])
> 
>  ;;
> @@ -14948,6 +14952,41 @@ (define_insn "*cmpeqb_internal"
>    "TARGET_P9_MISC && TARGET_64BIT"
>    "cmpeqb %0,%1,%2"
>    [(set_attr "type" "logical")])
> +
> +
> +;; ROP mitigation instructions.
> +
> +(define_insn "hashst"
> +  [(set (match_operand:DI 0 "simple_offsettable_mem_operand" "=m")
> +        (unspec:DI [(match_operand:DI 1 "int_reg_operand" "r")]
> +	           UNSPEC_HASHST))]
> +  "TARGET_POWER10 && rs6000_rop_protect"
> +  "hashst %1,%0"
> +  [(set_attr "type" "store")])
> +
> +(define_insn "hashstp"
> +  [(set (match_operand:DI 0 "simple_offsettable_mem_operand" "=m")
> +        (unspec:DI [(match_operand:DI 1 "int_reg_operand" "r")]
> +	           UNSPEC_HASHSTP))]
> +  "TARGET_POWER10 && rs6000_rop_protect && rs6000_privileged"
> +  "hashstp %1,%0"
> +  [(set_attr "type" "store")])
> +
> +(define_insn "hashchk"
> +  [(unspec_volatile [(match_operand:DI 0 "int_reg_operand" "r")
> +		     (match_operand:DI 1 "simple_offsettable_mem_operand" "m")]
> +		    UNSPEC_HASHCHK)]
> +  "TARGET_POWER10 && rs6000_rop_protect"
> +  "hashchk %0,%1"
> +  [(set_attr "type" "load")])
> +
> +(define_insn "hashchkp"
> +  [(unspec_volatile [(match_operand:DI 0 "int_reg_operand" "r")
> +		     (match_operand:DI 1 "simple_offsettable_mem_operand" "m")]
> +		    UNSPEC_HASHCHKP)]
> +  "TARGET_POWER10 && rs6000_rop_protect && rs6000_privileged"
> +  "hashchkp %0,%1"
> +  [(set_attr "type" "load")])

ok


lgtm, 
thanks,
-Will

>  
> 
>  (include "sync.md")