On Wed, Aug 3, 2022 at 10:27 AM H.J. Lu <hjl.to...@gmail.com> wrote: > > On Tue, Aug 2, 2022 at 4:34 PM Jeff Law <jeffreya...@gmail.com> wrote: > > > > > > > > On 8/2/2022 11:43 AM, H.J. Lu wrote: > > > On Sat, Jul 30, 2022 at 1:30 PM Jeff Law via Gcc-patches > > > <gcc-patches@gcc.gnu.org> wrote: > > >> > > >> > > >> On 7/14/2022 3:55 PM, H.J. Lu via Gcc-patches wrote: > > >>> Check stack canary for noreturn function to catch stack corruption > > >>> before calling noreturn function. For C++, check stack canary when > > >>> throwing exception or resuming stack unwind to avoid corrupted stack. > > >>> > > >>> gcc/ > > >>> > > >>> PR middle-end/58245 > > >>> * calls.cc (expand_call): Check stack canary for noreturn > > >>> function. > > >>> > > >>> gcc/testsuite/ > > >>> > > >>> PR middle-end/58245 > > >>> * c-c++-common/pr58245-1.c: New test. > > >>> * g++.dg/pr58245-1.C: Likewise. > > >>> * g++.dg/fstack-protector-strong.C: Adjusted. > > >> But is this really something we want? I'd actually lean towards > > >> eliminating the useless load -- I don't necessarily think we should be > > >> treating non-returning paths specially here. > > >> > > >> The whole point of the stack protector is to prevent the *return* path > > >> from going to an attacker controlled location. I'm not sure checking > > >> the protector at this point actually does anything particularly useful. > > > throw is marked no return. Since the unwind library may read > > > the stack contents to unwind stack, it the stack is corrupted, the > > > exception handling may go wrong. Should we handle this case? > > That's the question I think we need to answer. The EH paths are a known > > security issue on Windows and while ours are notably different I'm not > > sure if there's a real attack surface in those paths. My sense is that > > if we need to tackle this that doing so on the throw side might be > > better as it's closer conceptually to when//how we check the canary for > > a normal return. > > Like this? > > @@ -3154,7 +3155,10 @@ expand_call (tree exp, rtx target, int ignore) > if (pass && (flags & ECF_MALLOC)) > start_sequence (); > > - if (pass == 0 > + /* Check the canary value for sibcall or function which doesn't > + return and could throw. */ > + if ((pass == 0 > + || ((flags & ECF_NORETURN) != 0 && tree_could_throw_p (exp))) > && crtl->stack_protect_guard > && targetm.stack_protect_runtime_enabled_p ()) > stack_protect_epilogue ();
Here is the patch: https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599916.html > > jeff > > > > > > -- > > > H.J. > > > > > -- > H.J. -- H.J.
