Message ID | CAFiYyc2ZQvOy5eG2WMxjCisJ0ZgrKHnZw6d=5mieDAhLt2VRrQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
On 08/18/2017 08:01 AM, Richard Biener wrote: > On Mon, Jul 31, 2017 at 7:35 AM, Jeff Law <law@redhat.com> wrote: >> This patch introduces the stack clash protection options >> >> Changes since V2: >> >> Adds two new params. The first controls the size of the guard area. >> This controls the threshold for when a function prologue requires >> probes. The second controls the probing interval -- ie, once probes are >> needed, how often do we emit them. These are really meant more for >> developers to experiment with than users. Regardless I did go ahead and >> document them./PARAM >> >> It also adds some sanity checking WRT combining stack clash protection >> with -fstack-check. > > diff --git a/gcc/params.c b/gcc/params.c > index fab0ffa..8afe4c4 100644 > --- a/gcc/params.c > +++ b/gcc/params.c > @@ -209,6 +209,11 @@ set_param_value (const char *name, int value, > error ("maximum value of parameter %qs is %u", > compiler_params[i].option, > compiler_params[i].max_value); > + else if ((strcmp (name, "stack-clash-protection-guard-size") == 0 > + || strcmp (name, "stack-clash-protection-probe-interval") == 0) > + && exact_log2 (value) == -1) > + error ("value of parameter %qs must be a power of 2", > + compiler_params[i].option); > else > set_param_value_internal ((compiler_param) i, value, > params, params_set, true); > > I don't like this. Either use them as if they were power-of-two > (floor_log2/ceil_log2 as appropriate) or simply make them take > the logarithm instead (like -mincoming-stack-boundary and friends). Yes. I was torn on this for a variety of reasons, including the fact that I don't actually expect anyone to be mucking with those :-) Given we've already other stuff in log2 form, I'll use that -- it seems less surprising to me than using floor/ceil. > > Both -fstack-clash-protection and -fstack-check cannot be turned > off per function. This means they would need merging in lto-wrapper. > The alternative is to mark them with 'Optimization' and allow per-function > specification (like we do for -fstack-protector). Do you have a strong preference here? I'd tend to go with tweaking lto-wrapper as we really don't want to have this stuff varying per-function. Presumably in lto-wrapper we just have to detect that both were enabled and do something sensible. We drop -fstack-check in toplev.c when both are specified, we could just as easily call that situation a fatal error in both toplev.c and lto-wrapper.c jeff
On Tue, Aug 22, 2017 at 5:52 PM, Jeff Law <law@redhat.com> wrote: > On 08/18/2017 08:01 AM, Richard Biener wrote: >> On Mon, Jul 31, 2017 at 7:35 AM, Jeff Law <law@redhat.com> wrote: >>> This patch introduces the stack clash protection options >>> >>> Changes since V2: >>> >>> Adds two new params. The first controls the size of the guard area. >>> This controls the threshold for when a function prologue requires >>> probes. The second controls the probing interval -- ie, once probes are >>> needed, how often do we emit them. These are really meant more for >>> developers to experiment with than users. Regardless I did go ahead and >>> document them./PARAM >>> >>> It also adds some sanity checking WRT combining stack clash protection >>> with -fstack-check. >> >> diff --git a/gcc/params.c b/gcc/params.c >> index fab0ffa..8afe4c4 100644 >> --- a/gcc/params.c >> +++ b/gcc/params.c >> @@ -209,6 +209,11 @@ set_param_value (const char *name, int value, >> error ("maximum value of parameter %qs is %u", >> compiler_params[i].option, >> compiler_params[i].max_value); >> + else if ((strcmp (name, "stack-clash-protection-guard-size") == 0 >> + || strcmp (name, "stack-clash-protection-probe-interval") == 0) >> + && exact_log2 (value) == -1) >> + error ("value of parameter %qs must be a power of 2", >> + compiler_params[i].option); >> else >> set_param_value_internal ((compiler_param) i, value, >> params, params_set, true); >> >> I don't like this. Either use them as if they were power-of-two >> (floor_log2/ceil_log2 as appropriate) or simply make them take >> the logarithm instead (like -mincoming-stack-boundary and friends). > Yes. I was torn on this for a variety of reasons, including the fact > that I don't actually expect anyone to be mucking with those :-) > > Given we've already other stuff in log2 form, I'll use that -- it seems > less surprising to me than using floor/ceil. > >> >> Both -fstack-clash-protection and -fstack-check cannot be turned >> off per function. This means they would need merging in lto-wrapper. >> The alternative is to mark them with 'Optimization' and allow per-function >> specification (like we do for -fstack-protector). > Do you have a strong preference here? I'd tend to go with tweaking > lto-wrapper as we really don't want to have this stuff varying per-function. Any reason? I see no technical one at least, it might break IPA assumptions you make when instrumenting? I guess you just don't want to think about the implications when mixing them (or mixing -fstack-clash-protection with -fno-stack-clash-protection)? > Presumably in lto-wrapper we just have to detect that both were enabled > and do something sensible. We drop -fstack-check in toplev.c when both > are specified, we could just as easily call that situation a fatal error > in both toplev.c and lto-wrapper.c So what happens if you LTO link Ada (-fstack-check) with C (-fstack-clash-protection)? The user is already allowed to (without LTO) specify things on a CU granularity. Richard. > > jeff > >
On 08/23/2017 03:07 AM, Richard Biener wrote: >>> Both -fstack-clash-protection and -fstack-check cannot be turned >>> off per function. This means they would need merging in lto-wrapper. >>> The alternative is to mark them with 'Optimization' and allow per-function >>> specification (like we do for -fstack-protector). >> Do you have a strong preference here? I'd tend to go with tweaking >> lto-wrapper as we really don't want to have this stuff varying per-function. > > Any reason? I see no technical one at least, it might break IPA assumptions > you make when instrumenting? I guess you just don't want to think about > the implications when mixing them (or mixing -fstack-clash-protection > with -fno-stack-clash-protection)? On some targets (s390 for example), a stack guard jump can be accomplished by a combination of caller and callee stack adjustments -- even though neither the caller nor the callee have a large enough adjustment individually to jump the guard. So a mis-informed developer could audit their code, declare various functions as safe & turn off protections on a per-function basis based on that analysis. But that would be mis-guided as it opens them up to a potential attack where the combination of caller and callee stack adjustments are used to jump the guard. > >> Presumably in lto-wrapper we just have to detect that both were enabled >> and do something sensible. We drop -fstack-check in toplev.c when both >> are specified, we could just as easily call that situation a fatal error >> in both toplev.c and lto-wrapper.c > > So what happens if you LTO link Ada (-fstack-check) with C > (-fstack-clash-protection)? When in the Ada code you'll be mostly protected from stack clashes and you can detect stack overflows and catch the signal properly. When in C code, you'd be fully protected from stack clash, but not necessarily able to take a signal. At the boundaries where you call from Ada to C, you're potentially vulnerable to stack clash on architectures like aarch64/s390. When calling from C into Ada, you're not guaranteed to be able to handle a signal when you overflow the stack. > The user is already allowed to (without LTO) specify things on a CU granularity. True. And you end up with a mixture of protections depending on what code is executing or what boundary you're crossing. Jeff
On August 23, 2017 6:36:32 PM GMT+02:00, Jeff Law <law@redhat.com> wrote: >On 08/23/2017 03:07 AM, Richard Biener wrote: > >>>> Both -fstack-clash-protection and -fstack-check cannot be turned >>>> off per function. This means they would need merging in >lto-wrapper. >>>> The alternative is to mark them with 'Optimization' and allow >per-function >>>> specification (like we do for -fstack-protector). >>> Do you have a strong preference here? I'd tend to go with tweaking >>> lto-wrapper as we really don't want to have this stuff varying >per-function. >> >> Any reason? I see no technical one at least, it might break IPA >assumptions >> you make when instrumenting? I guess you just don't want to think >about >> the implications when mixing them (or mixing -fstack-clash-protection >> with -fno-stack-clash-protection)? >On some targets (s390 for example), a stack guard jump can be >accomplished by a combination of caller and callee stack adjustments -- >even though neither the caller nor the callee have a large enough >adjustment individually to jump the guard. > >So a mis-informed developer could audit their code, declare various >functions as safe & turn off protections on a per-function basis based >on that analysis. But that would be mis-guided as it opens them up to >a >potential attack where the combination of caller and callee stack >adjustments are used to jump the guard. > > > > > >> >>> Presumably in lto-wrapper we just have to detect that both were >enabled >>> and do something sensible. We drop -fstack-check in toplev.c when >both >>> are specified, we could just as easily call that situation a fatal >error >>> in both toplev.c and lto-wrapper.c >> >> So what happens if you LTO link Ada (-fstack-check) with C >> (-fstack-clash-protection)? >When in the Ada code you'll be mostly protected from stack clashes and >you can detect stack overflows and catch the signal properly. When in >C >code, you'd be fully protected from stack clash, but not necessarily >able to take a signal. > >At the boundaries where you call from Ada to C, you're potentially >vulnerable to stack clash on architectures like aarch64/s390. When >calling from C into Ada, you're not guaranteed to be able to handle a >signal when you overflow the stack. > >> The user is already allowed to (without LTO) specify things on a CU >granularity. >True. And you end up with a mixture of protections depending on what >code is executing or what boundary you're crossing. So the natural thing for LTO ist to make it function granular. That way you don't need any magic in LTO-wrapper. Richard. > >Jeff
On 08/23/2017 10:39 AM, Richard Biener wrote: > On August 23, 2017 6:36:32 PM GMT+02:00, Jeff Law <law@redhat.com> wrote: >> On 08/23/2017 03:07 AM, Richard Biener wrote: >> >>>>> Both -fstack-clash-protection and -fstack-check cannot be turned >>>>> off per function. This means they would need merging in >> lto-wrapper. >>>>> The alternative is to mark them with 'Optimization' and allow >> per-function >>>>> specification (like we do for -fstack-protector). >>>> Do you have a strong preference here? I'd tend to go with tweaking >>>> lto-wrapper as we really don't want to have this stuff varying >> per-function. >>> >>> Any reason? I see no technical one at least, it might break IPA >> assumptions >>> you make when instrumenting? I guess you just don't want to think >> about >>> the implications when mixing them (or mixing -fstack-clash-protection >>> with -fno-stack-clash-protection)? >> On some targets (s390 for example), a stack guard jump can be >> accomplished by a combination of caller and callee stack adjustments -- >> even though neither the caller nor the callee have a large enough >> adjustment individually to jump the guard. >> >> So a mis-informed developer could audit their code, declare various >> functions as safe & turn off protections on a per-function basis based >> on that analysis. But that would be mis-guided as it opens them up to >> a >> potential attack where the combination of caller and callee stack >> adjustments are used to jump the guard. >> >> >> >> >> >>> >>>> Presumably in lto-wrapper we just have to detect that both were >> enabled >>>> and do something sensible. We drop -fstack-check in toplev.c when >> both >>>> are specified, we could just as easily call that situation a fatal >> error >>>> in both toplev.c and lto-wrapper.c >>> >>> So what happens if you LTO link Ada (-fstack-check) with C >>> (-fstack-clash-protection)? >> When in the Ada code you'll be mostly protected from stack clashes and >> you can detect stack overflows and catch the signal properly. When in >> C >> code, you'd be fully protected from stack clash, but not necessarily >> able to take a signal. >> >> At the boundaries where you call from Ada to C, you're potentially >> vulnerable to stack clash on architectures like aarch64/s390. When >> calling from C into Ada, you're not guaranteed to be able to handle a >> signal when you overflow the stack. >> >>> The user is already allowed to (without LTO) specify things on a CU >> granularity. >> True. And you end up with a mixture of protections depending on what >> code is executing or what boundary you're crossing. > > So the natural thing for LTO ist to make it function granular. That way you don't need any magic in LTO-wrapper. Done. jeff
diff --git a/gcc/params.c b/gcc/params.c index fab0ffa..8afe4c4 100644 --- a/gcc/params.c +++ b/gcc/params.c @@ -209,6 +209,11 @@ set_param_value (const char *name, int value, error ("maximum value of parameter %qs is %u", compiler_params[i].option, compiler_params[i].max_value); + else if ((strcmp (name, "stack-clash-protection-guard-size") == 0 + || strcmp (name, "stack-clash-protection-probe-interval") == 0) + && exact_log2 (value) == -1) + error ("value of parameter %qs must be a power of 2", + compiler_params[i].option); else set_param_value_internal ((compiler_param) i, value, params, params_set, true);