[PATCH 0/2] gcc-plugins: fixes for arm_ssp_per_task_plugin

[PATCH 0/2] gcc-plugins: fixes for arm_ssp_per_task_plugin

[Ard Biesheuvel]

A couple of fixes to permit newer versions of GCC to use the stack
protector plugin for ARM.

Ard Biesheuvel (2):
  gcc-plugins: arm_ssp_per_task_plugin: sign extend the SP mask
  gcc-plugins: arm_ssp_per_task_plugin: fix for GCC 9+

 scripts/gcc-plugins/arm_ssp_per_task_plugin.c | 23 ++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

-- 
2.20.1

[PATCH 1/2] gcc-plugins: arm_ssp_per_task_plugin: sign extend the SP mask

[Ard Biesheuvel]

The ARM per-task stack protector GCC plugin hits an assert in
the compiler in some case, due to the fact the the SP mask
expression is not sign-extended as it should be. So fix that.

Suggested-by: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/gcc-plugins/arm_ssp_per_task_plugin.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/gcc-plugins/arm_ssp_per_task_plugin.c b/scripts/gcc-plugins/arm_ssp_per_task_plugin.c
index de70b8470971..a65fbefb8501 100644
--- a/scripts/gcc-plugins/arm_ssp_per_task_plugin.c
+++ b/scripts/gcc-plugins/arm_ssp_per_task_plugin.c
@@ -13,7 +13,7 @@ static unsigned int arm_pertask_ssp_rtl_execute(void)
 	for (insn = get_insns(); insn; insn = NEXT_INSN(insn)) {
 		const char *sym;
 		rtx body;
-		rtx masked_sp;
+		rtx mask, masked_sp;
 
 		/*
 		 * Find a SET insn involving a SYMBOL_REF to __stack_chk_guard
@@ -33,12 +33,13 @@ static unsigned int arm_pertask_ssp_rtl_execute(void)
 		 * produces the address of the copy of the stack canary value
 		 * stored in struct thread_info
 		 */
+		mask = GEN_INT(sext_hwi(sp_mask, GET_MODE_PRECISION(Pmode)));
 		masked_sp = gen_reg_rtx(Pmode);
 
 		emit_insn_before(gen_rtx_SET(masked_sp,
 					     gen_rtx_AND(Pmode,
 							 stack_pointer_rtx,
-							 GEN_INT(sp_mask))),
+							 mask)),
 				 insn);
 
 		SET_SRC(body) = gen_rtx_PLUS(Pmode, masked_sp,
-- 
2.20.1

[PATCH 2/2] gcc-plugins: arm_ssp_per_task_plugin: fix for GCC 9+

[Ard Biesheuvel]

GCC 9 reworks the way the references to the stack canary are
emitted, to prevent the value from being spilled to the stack
before the final comparison in the epilogue, defeating the
purpose, given that the spill slot is under control of the
attacker that we are protecting ourselves from.

Since our canary value address is obtained without accessing
memory (as opposed to pre-v7 code that will obtain it from a
literal pool), it is unlikely (although not guaranteed) that
the compiler will spill the canary value in the same way, so
let's just disable this improvement when building with GCC9+.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/gcc-plugins/arm_ssp_per_task_plugin.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/scripts/gcc-plugins/arm_ssp_per_task_plugin.c b/scripts/gcc-plugins/arm_ssp_per_task_plugin.c
index a65fbefb8501..89c47f57d1ce 100644
--- a/scripts/gcc-plugins/arm_ssp_per_task_plugin.c
+++ b/scripts/gcc-plugins/arm_ssp_per_task_plugin.c
@@ -53,6 +53,19 @@ static unsigned int arm_pertask_ssp_rtl_execute(void)
 #define NO_GATE
 #include "gcc-generate-rtl-pass.h"
 
+#if BUILDING_GCC_VERSION >= 9000
+static bool no(void)
+{
+	return false;
+}
+
+static void arm_pertask_ssp_start_unit(void *gcc_data, void *user_data)
+{
+	targetm.have_stack_protect_combined_set = no;
+	targetm.have_stack_protect_combined_test = no;
+}
+#endif
+
 __visible int plugin_init(struct plugin_name_args *plugin_info,
 			  struct plugin_gcc_version *version)
 {
@@ -100,5 +113,10 @@ __visible int plugin_init(struct plugin_name_args *plugin_info,
 	register_callback(plugin_info->base_name, PLUGIN_PASS_MANAGER_SETUP,
 			  NULL, &arm_pertask_ssp_rtl_pass_info);
 
+#if BUILDING_GCC_VERSION >= 9000
+	register_callback(plugin_info->base_name, PLUGIN_START_UNIT,
+			  arm_pertask_ssp_start_unit, NULL);
+#endif
+
 	return 0;
 }
-- 
2.20.1

Re: [PATCH 0/2] gcc-plugins: fixes for arm_ssp_per_task_plugin

[Kees Cook]

On Fri, Jan 18, 2019 at 2:58 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> A couple of fixes to permit newer versions of GCC to use the stack
> protector plugin for ARM.
>
> Ard Biesheuvel (2):
>   gcc-plugins: arm_ssp_per_task_plugin: sign extend the SP mask
>   gcc-plugins: arm_ssp_per_task_plugin: fix for GCC 9+

Nice; thanks! It seems like these should go into -rc4, yes? I'll get
them queued up for Linus...

-Kees

>
>  scripts/gcc-plugins/arm_ssp_per_task_plugin.c | 23 ++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> --
> 2.20.1
>


-- 
Kees Cook

Re: [PATCH 0/2] gcc-plugins: fixes for arm_ssp_per_task_plugin

[Ard Biesheuvel]

On Sun, 20 Jan 2019 at 02:51, Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jan 18, 2019 at 2:58 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > A couple of fixes to permit newer versions of GCC to use the stack
> > protector plugin for ARM.
> >
> > Ard Biesheuvel (2):
> >   gcc-plugins: arm_ssp_per_task_plugin: sign extend the SP mask
> >   gcc-plugins: arm_ssp_per_task_plugin: fix for GCC 9+
>
> Nice; thanks! It seems like these should go into -rc4, yes? I'll get
> them queued up for Linus...
>

Yes, given that it is new code anyway, let's merge it as fixes.