Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access
Hi Cooper: Thanks for your patch! committed to trunk. https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=c931e8d5a96463427040b0d11f9c4352ac22b2b0 On Wed, Jul 29, 2020 at 8:34 PM Cooper Qu via Gcc-patches wrote: > > Sorry for later replay, I will add testcases on a following patch if the > patch is accepted. > > > Regards, > > Cooper > > On 2020/7/28 上午9:23, Kito Cheng wrote: > > Add testcase later is OK to me. > > > > On Tue, Jul 28, 2020 at 6:55 AM Jim Wilson wrote: > >> On Sun, Jul 19, 2020 at 7:04 PM cooper wrote: > >>> Ping > >>> > >>> On 2020/7/13 下午4:15, cooper wrote: > gcc/ > * config/riscv/riscv-opts.h (stack_protector_guard): New enum. > * config/riscv/riscv.c (riscv_option_override): Handle > the new options. > * config/riscv/riscv.md (stack_protect_set): New pattern to handle > flexible stack protector guard settings. > (stack_protect_set_): Ditto. > (stack_protect_test): Ditto. > (stack_protect_test_): Ditto. > * config/riscv/riscv.opt (mstack-protector-guard=, > mstack-protector-guard-reg=, mstack-protector-guard-offset=): New > options. > * doc/invoke.texi (Option Summary) [RISC-V Options]: > Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and > -mstack-protector-guard-offset=. > (RISC-V Options): Ditto. > >> The v2 patch looks fine to me. Meanwhile, Kito asked for testcases > >> which would be nice to have but I don't think is critical considering > >> that this has already been tested with a kernel build. Maybe the > >> testcases can be a follow on patch? I'd like to see forward movement > >> on this, even if we accept a patch without the testcases. > >> > >> Jim
Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access
Sorry for later replay, I will add testcases on a following patch if the patch is accepted. Regards, Cooper On 2020/7/28 上午9:23, Kito Cheng wrote: Add testcase later is OK to me. On Tue, Jul 28, 2020 at 6:55 AM Jim Wilson wrote: On Sun, Jul 19, 2020 at 7:04 PM cooper wrote: Ping On 2020/7/13 下午4:15, cooper wrote: gcc/ * config/riscv/riscv-opts.h (stack_protector_guard): New enum. * config/riscv/riscv.c (riscv_option_override): Handle the new options. * config/riscv/riscv.md (stack_protect_set): New pattern to handle flexible stack protector guard settings. (stack_protect_set_): Ditto. (stack_protect_test): Ditto. (stack_protect_test_): Ditto. * config/riscv/riscv.opt (mstack-protector-guard=, mstack-protector-guard-reg=, mstack-protector-guard-offset=): New options. * doc/invoke.texi (Option Summary) [RISC-V Options]: Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and -mstack-protector-guard-offset=. (RISC-V Options): Ditto. The v2 patch looks fine to me. Meanwhile, Kito asked for testcases which would be nice to have but I don't think is critical considering that this has already been tested with a kernel build. Maybe the testcases can be a follow on patch? I'd like to see forward movement on this, even if we accept a patch without the testcases. Jim
Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access
Add testcase later is OK to me. On Tue, Jul 28, 2020 at 6:55 AM Jim Wilson wrote: > > On Sun, Jul 19, 2020 at 7:04 PM cooper wrote: > > Ping > > > > On 2020/7/13 下午4:15, cooper wrote: > > > gcc/ > > > * config/riscv/riscv-opts.h (stack_protector_guard): New enum. > > > * config/riscv/riscv.c (riscv_option_override): Handle > > > the new options. > > > * config/riscv/riscv.md (stack_protect_set): New pattern to handle > > > flexible stack protector guard settings. > > > (stack_protect_set_): Ditto. > > > (stack_protect_test): Ditto. > > > (stack_protect_test_): Ditto. > > > * config/riscv/riscv.opt (mstack-protector-guard=, > > > mstack-protector-guard-reg=, mstack-protector-guard-offset=): New > > > options. > > > * doc/invoke.texi (Option Summary) [RISC-V Options]: > > > Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and > > > -mstack-protector-guard-offset=. > > > (RISC-V Options): Ditto. > > The v2 patch looks fine to me. Meanwhile, Kito asked for testcases > which would be nice to have but I don't think is critical considering > that this has already been tested with a kernel build. Maybe the > testcases can be a follow on patch? I'd like to see forward movement > on this, even if we accept a patch without the testcases. > > Jim
Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access
On Sun, Jul 19, 2020 at 7:04 PM cooper wrote: > Ping > > On 2020/7/13 下午4:15, cooper wrote: > > gcc/ > > * config/riscv/riscv-opts.h (stack_protector_guard): New enum. > > * config/riscv/riscv.c (riscv_option_override): Handle > > the new options. > > * config/riscv/riscv.md (stack_protect_set): New pattern to handle > > flexible stack protector guard settings. > > (stack_protect_set_): Ditto. > > (stack_protect_test): Ditto. > > (stack_protect_test_): Ditto. > > * config/riscv/riscv.opt (mstack-protector-guard=, > > mstack-protector-guard-reg=, mstack-protector-guard-offset=): New > > options. > > * doc/invoke.texi (Option Summary) [RISC-V Options]: > > Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and > > -mstack-protector-guard-offset=. > > (RISC-V Options): Ditto. The v2 patch looks fine to me. Meanwhile, Kito asked for testcases which would be nice to have but I don't think is critical considering that this has already been tested with a kernel build. Maybe the testcases can be a follow on patch? I'd like to see forward movement on this, even if we accept a patch without the testcases. Jim
Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access
Hi Cooper:
Could you add testcases like ppc[3-4]?
[3]
https://github.com/gcc-mirror/gcc/blob/master/gcc/testsuite/gcc.target/powerpc/ssp-1.c
[4]
https://github.com/gcc-mirror/gcc/blob/master/gcc/testsuite/gcc.target/powerpc/ssp-2.c
On Mon, Jul 20, 2020 at 10:04 AM cooper via Gcc-patches
wrote:
>
> Ping
>
> On 2020/7/13 下午4:15, cooper wrote:
> > gcc/
> > * config/riscv/riscv-opts.h (stack_protector_guard): New enum.
> > * config/riscv/riscv.c (riscv_option_override): Handle
> > the new options.
> > * config/riscv/riscv.md (stack_protect_set): New pattern to handle
> > flexible stack protector guard settings.
> > (stack_protect_set_): Ditto.
> > (stack_protect_test): Ditto.
> > (stack_protect_test_): Ditto.
> > * config/riscv/riscv.opt (mstack-protector-guard=,
> > mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
> > options.
> > * doc/invoke.texi (Option Summary) [RISC-V Options]:
> > Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
> > -mstack-protector-guard-offset=.
> > (RISC-V Options): Ditto.
> >
> > Signed-off-by: cooper
> > Signed-off-by: Guo Ren
> > ---
> > gcc/config/riscv/riscv-opts.h | 6 +++
> > gcc/config/riscv/riscv.c | 47
> > gcc/config/riscv/riscv.md | 80 +++
> > gcc/config/riscv/riscv.opt| 28
> > gcc/doc/invoke.texi | 22 +-
> > 5 files changed, 182 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
> > index 8f12e50b9f1..2a3f9d9eef5 100644
> > --- a/gcc/config/riscv/riscv-opts.h
> > +++ b/gcc/config/riscv/riscv-opts.h
> > @@ -51,4 +51,10 @@ enum riscv_align_data {
> > riscv_align_data_type_natural
> > };
> >
> > +/* Where to get the canary for the stack protector. */
> > +enum stack_protector_guard {
> > + SSP_TLS, /* per-thread canary in TLS block */
> > + SSP_GLOBAL /* global canary */
> > +};
> > +
> > #endif /* ! GCC_RISCV_OPTS_H */
> > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> > index bfb3885ed08..63b0c3877b0 100644
> > --- a/gcc/config/riscv/riscv.c
> > +++ b/gcc/config/riscv/riscv.c
> > @@ -4775,6 +4775,53 @@ riscv_option_override (void)
> > " [%<-mriscv-attribute%>]");
> > #endif
> >
> > + if (riscv_stack_protector_guard == SSP_GLOBAL
> > + && global_options_set.x_riscv_stack_protector_guard_offset_str)
> > +{
> > + error ("incompatible options %<-mstack-protector-guard=global%> and "
> > + "%<-mstack-protector-guard-offset=%s%>",
> > + riscv_stack_protector_guard_offset_str);
> > +}
> > +
> > + if (riscv_stack_protector_guard == SSP_TLS
> > + && !(global_options_set.x_riscv_stack_protector_guard_offset_str
> > +&& global_options_set.x_riscv_stack_protector_guard_reg_str))
> > +{
> > + error ("both %<-mstack-protector-guard-offset%> and "
> > + "%<-mstack-protector-guard-reg%> must be used "
> > + "with %<-mstack-protector-guard=sysreg%>");
> > +}
> > +
> > + if (global_options_set.x_riscv_stack_protector_guard_reg_str)
> > +{
> > + const char *str = riscv_stack_protector_guard_reg_str;
> > + int reg = decode_reg_name (str);
> > +
> > + if (!IN_RANGE (reg, GP_REG_FIRST + 1, GP_REG_LAST))
> > + error ("%qs is not a valid base register in %qs", str,
> > +"-mstack-protector-guard-reg=");
> > +
> > + riscv_stack_protector_guard_reg = reg;
> > +}
> > +
> > + if (global_options_set.x_riscv_stack_protector_guard_offset_str)
> > +{
> > + char *end;
> > + const char *str = riscv_stack_protector_guard_offset_str;
> > + errno = 0;
> > + long offs = strtol (riscv_stack_protector_guard_offset_str, &end, 0);
> > +
> > + if (!*str || *end || errno)
> > + error ("%qs is not a valid number in %qs", str,
> > +"-mstack-protector-guard-offset=");
> > +
> > + if (!SMALL_OPERAND (offs))
> > + error ("%qs is not a valid offset in %qs", str,
> > +"-mstack-protector-guard-offset=");
> > +
> > + riscv_stack_protector_guard_offset = offs;
> > +}
> > +
> > }
> >
> > /* Implement TARGET_CONDITIONAL_REGISTER_USAGE. */
> > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> > index 95a02ecaa34..f15bad3b29e 100644
> > --- a/gcc/config/riscv/riscv.md
> > +++ b/gcc/config/riscv/riscv.md
> > @@ -65,6 +65,10 @@
> > UNSPECV_BLOCKAGE
> > UNSPECV_FENCE
> > UNSPECV_FENCE_I
> > +
> > + ;; Stack Smash Protector
> > + UNSPEC_SSP_SET
> > + UNSPEC_SSP_TEST
> > ])
> >
> > (define_constants
> > @@ -2523,6 +2527,82 @@
> > ""
> > {})
> >
> > +;; Named patterns for stack smashing protection.
> > +
> > +(define_expand "stack_protect_set"
> > + [(match_operand 0 "memory_operand")
> > + (match_operand 1 "memory_o
Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access
Ping
On 2020/7/13 下午4:15, cooper wrote:
gcc/
* config/riscv/riscv-opts.h (stack_protector_guard): New enum.
* config/riscv/riscv.c (riscv_option_override): Handle
the new options.
* config/riscv/riscv.md (stack_protect_set): New pattern to handle
flexible stack protector guard settings.
(stack_protect_set_): Ditto.
(stack_protect_test): Ditto.
(stack_protect_test_): Ditto.
* config/riscv/riscv.opt (mstack-protector-guard=,
mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
options.
* doc/invoke.texi (Option Summary) [RISC-V Options]:
Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
-mstack-protector-guard-offset=.
(RISC-V Options): Ditto.
Signed-off-by: cooper
Signed-off-by: Guo Ren
---
gcc/config/riscv/riscv-opts.h | 6 +++
gcc/config/riscv/riscv.c | 47
gcc/config/riscv/riscv.md | 80 +++
gcc/config/riscv/riscv.opt| 28
gcc/doc/invoke.texi | 22 +-
5 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
index 8f12e50b9f1..2a3f9d9eef5 100644
--- a/gcc/config/riscv/riscv-opts.h
+++ b/gcc/config/riscv/riscv-opts.h
@@ -51,4 +51,10 @@ enum riscv_align_data {
riscv_align_data_type_natural
};
+/* Where to get the canary for the stack protector. */
+enum stack_protector_guard {
+ SSP_TLS, /* per-thread canary in TLS block */
+ SSP_GLOBAL /* global canary */
+};
+
#endif /* ! GCC_RISCV_OPTS_H */
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index bfb3885ed08..63b0c3877b0 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -4775,6 +4775,53 @@ riscv_option_override (void)
" [%<-mriscv-attribute%>]");
#endif
+ if (riscv_stack_protector_guard == SSP_GLOBAL
+ && global_options_set.x_riscv_stack_protector_guard_offset_str)
+{
+ error ("incompatible options %<-mstack-protector-guard=global%> and "
+"%<-mstack-protector-guard-offset=%s%>",
+riscv_stack_protector_guard_offset_str);
+}
+
+ if (riscv_stack_protector_guard == SSP_TLS
+ && !(global_options_set.x_riscv_stack_protector_guard_offset_str
+ && global_options_set.x_riscv_stack_protector_guard_reg_str))
+{
+ error ("both %<-mstack-protector-guard-offset%> and "
+"%<-mstack-protector-guard-reg%> must be used "
+"with %<-mstack-protector-guard=sysreg%>");
+}
+
+ if (global_options_set.x_riscv_stack_protector_guard_reg_str)
+{
+ const char *str = riscv_stack_protector_guard_reg_str;
+ int reg = decode_reg_name (str);
+
+ if (!IN_RANGE (reg, GP_REG_FIRST + 1, GP_REG_LAST))
+ error ("%qs is not a valid base register in %qs", str,
+ "-mstack-protector-guard-reg=");
+
+ riscv_stack_protector_guard_reg = reg;
+}
+
+ if (global_options_set.x_riscv_stack_protector_guard_offset_str)
+{
+ char *end;
+ const char *str = riscv_stack_protector_guard_offset_str;
+ errno = 0;
+ long offs = strtol (riscv_stack_protector_guard_offset_str, &end, 0);
+
+ if (!*str || *end || errno)
+ error ("%qs is not a valid number in %qs", str,
+ "-mstack-protector-guard-offset=");
+
+ if (!SMALL_OPERAND (offs))
+ error ("%qs is not a valid offset in %qs", str,
+ "-mstack-protector-guard-offset=");
+
+ riscv_stack_protector_guard_offset = offs;
+}
+
}
/* Implement TARGET_CONDITIONAL_REGISTER_USAGE. */
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 95a02ecaa34..f15bad3b29e 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -65,6 +65,10 @@
UNSPECV_BLOCKAGE
UNSPECV_FENCE
UNSPECV_FENCE_I
+
+ ;; Stack Smash Protector
+ UNSPEC_SSP_SET
+ UNSPEC_SSP_TEST
])
(define_constants
@@ -2523,6 +2527,82 @@
""
{})
+;; Named patterns for stack smashing protection.
+
+(define_expand "stack_protect_set"
+ [(match_operand 0 "memory_operand")
+ (match_operand 1 "memory_operand")]
+ ""
+{
+ machine_mode mode = GET_MODE (operands[0]);
+ if (riscv_stack_protector_guard == SSP_TLS)
+ {
+rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
+rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
+rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
+operands[1] = gen_rtx_MEM (Pmode, addr);
+ }
+
+ emit_insn ((mode == DImode
+ ? gen_stack_protect_set_di
+ : gen_stack_protect_set_si) (operands[0], operands[1]));
+ DONE;
+})
+
+;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the
+;; canary value does not live beyond the life of this sequence.
+(define_insn "stack_protect_set_"
+ [(set (match_operand:GPR 0 "m
[PATCH v2] [RISC-V] Add support for TLS stack protector canary access
gcc/
* config/riscv/riscv-opts.h (stack_protector_guard): New enum.
* config/riscv/riscv.c (riscv_option_override): Handle
the new options.
* config/riscv/riscv.md (stack_protect_set): New pattern to handle
flexible stack protector guard settings.
(stack_protect_set_): Ditto.
(stack_protect_test): Ditto.
(stack_protect_test_): Ditto.
* config/riscv/riscv.opt (mstack-protector-guard=,
mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
options.
* doc/invoke.texi (Option Summary) [RISC-V Options]:
Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
-mstack-protector-guard-offset=.
(RISC-V Options): Ditto.
Signed-off-by: cooper
Signed-off-by: Guo Ren
---
gcc/config/riscv/riscv-opts.h | 6 +++
gcc/config/riscv/riscv.c | 47
gcc/config/riscv/riscv.md | 80 +++
gcc/config/riscv/riscv.opt| 28
gcc/doc/invoke.texi | 22 +-
5 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
index 8f12e50b9f1..2a3f9d9eef5 100644
--- a/gcc/config/riscv/riscv-opts.h
+++ b/gcc/config/riscv/riscv-opts.h
@@ -51,4 +51,10 @@ enum riscv_align_data {
riscv_align_data_type_natural
};
+/* Where to get the canary for the stack protector. */
+enum stack_protector_guard {
+ SSP_TLS, /* per-thread canary in TLS block */
+ SSP_GLOBAL /* global canary */
+};
+
#endif /* ! GCC_RISCV_OPTS_H */
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index bfb3885ed08..63b0c3877b0 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -4775,6 +4775,53 @@ riscv_option_override (void)
" [%<-mriscv-attribute%>]");
#endif
+ if (riscv_stack_protector_guard == SSP_GLOBAL
+ && global_options_set.x_riscv_stack_protector_guard_offset_str)
+{
+ error ("incompatible options %<-mstack-protector-guard=global%> and "
+"%<-mstack-protector-guard-offset=%s%>",
+riscv_stack_protector_guard_offset_str);
+}
+
+ if (riscv_stack_protector_guard == SSP_TLS
+ && !(global_options_set.x_riscv_stack_protector_guard_offset_str
+ && global_options_set.x_riscv_stack_protector_guard_reg_str))
+{
+ error ("both %<-mstack-protector-guard-offset%> and "
+"%<-mstack-protector-guard-reg%> must be used "
+"with %<-mstack-protector-guard=sysreg%>");
+}
+
+ if (global_options_set.x_riscv_stack_protector_guard_reg_str)
+{
+ const char *str = riscv_stack_protector_guard_reg_str;
+ int reg = decode_reg_name (str);
+
+ if (!IN_RANGE (reg, GP_REG_FIRST + 1, GP_REG_LAST))
+ error ("%qs is not a valid base register in %qs", str,
+ "-mstack-protector-guard-reg=");
+
+ riscv_stack_protector_guard_reg = reg;
+}
+
+ if (global_options_set.x_riscv_stack_protector_guard_offset_str)
+{
+ char *end;
+ const char *str = riscv_stack_protector_guard_offset_str;
+ errno = 0;
+ long offs = strtol (riscv_stack_protector_guard_offset_str, &end, 0);
+
+ if (!*str || *end || errno)
+ error ("%qs is not a valid number in %qs", str,
+ "-mstack-protector-guard-offset=");
+
+ if (!SMALL_OPERAND (offs))
+ error ("%qs is not a valid offset in %qs", str,
+ "-mstack-protector-guard-offset=");
+
+ riscv_stack_protector_guard_offset = offs;
+}
+
}
/* Implement TARGET_CONDITIONAL_REGISTER_USAGE. */
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 95a02ecaa34..f15bad3b29e 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -65,6 +65,10 @@
UNSPECV_BLOCKAGE
UNSPECV_FENCE
UNSPECV_FENCE_I
+
+ ;; Stack Smash Protector
+ UNSPEC_SSP_SET
+ UNSPEC_SSP_TEST
])
(define_constants
@@ -2523,6 +2527,82 @@
""
{})
+;; Named patterns for stack smashing protection.
+
+(define_expand "stack_protect_set"
+ [(match_operand 0 "memory_operand")
+ (match_operand 1 "memory_operand")]
+ ""
+{
+ machine_mode mode = GET_MODE (operands[0]);
+ if (riscv_stack_protector_guard == SSP_TLS)
+ {
+rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
+rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
+rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
+operands[1] = gen_rtx_MEM (Pmode, addr);
+ }
+
+ emit_insn ((mode == DImode
+ ? gen_stack_protect_set_di
+ : gen_stack_protect_set_si) (operands[0], operands[1]));
+ DONE;
+})
+
+;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the
+;; canary value does not live beyond the life of this sequence.
+(define_insn "stack_protect_set_"
+ [(set (match_operand:GPR 0 "memory_operand" "=m")
+ (unspec:GPR [(match_operand:GPR 1 "memory_o
