* [PATCH] x86: Check corrupted return address when unwinding stack
@ 2022-09-21 20:42 H.J. Lu
2022-09-30 19:33 ` Jeff Law
2022-10-04 21:32 ` PING^1: " H.J. Lu
0 siblings, 2 replies; 4+ messages in thread
From: H.J. Lu @ 2022-09-21 20:42 UTC (permalink / raw)
To: gcc-patches; +Cc: Uros Bizjak
If shadow stack is enabled, when unwinding stack, we count how many stack
frames we pop to reach the landing pad and adjust shadow stack by the same
amount. When counting the stack frame, we compare the return address on
normal stack against the return address on shadow stack. If they don't
match, return _URC_FATAL_PHASE2_ERROR for the corrupted return address on
normal stack. Don't check the return address for
1. Non-catchable exception where exception_class == 0. Process will be
terminated.
2. Zero return address which marks the outermost stack frame.
3. Signal stack frame since kernel puts a restore token on shadow stack.
* unwind-generic.h (_Unwind_Frames_Increment): Add the EXC
argument.
* unwind.inc (_Unwind_RaiseException_Phase2): Pass EXC to
_Unwind_Frames_Increment.
(_Unwind_ForcedUnwind_Phase2): Likewise.
* config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
Take the EXC argument. Return _URC_FATAL_PHASE2_ERROR if the
return address on normal stack doesn't match the return address
on shadow stack.
---
libgcc/config/i386/shadow-stack-unwind.h | 51 ++++++++++++++++++++++--
libgcc/unwind-generic.h | 2 +-
libgcc/unwind.inc | 4 +-
3 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h
index 2b02682bdae..89d44165000 100644
--- a/libgcc/config/i386/shadow-stack-unwind.h
+++ b/libgcc/config/i386/shadow-stack-unwind.h
@@ -54,10 +54,39 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
aligned. If the original shadow stack is 8 byte aligned, we just
need to pop 2 slots, one restore token, from shadow stack. Otherwise,
we need to pop 3 slots, one restore token + 4 byte padding, from
- shadow stack. */
-#ifndef __x86_64__
+ shadow stack.
+
+ When popping a stack frame, we compare the return address on normal
+ stack against the return address on shadow stack. If they don't match,
+ return _URC_FATAL_PHASE2_ERROR for the corrupted return address on
+ normal stack. Don't check the return address for
+ 1. Non-catchable exception where exception_class == 0. Process will
+ be terminated.
+ 2. Zero return address which marks the outermost stack frame.
+ 3. Signal stack frame since kernel puts a restore token on shadow
+ stack.
+ */
#undef _Unwind_Frames_Increment
-#define _Unwind_Frames_Increment(context, frames) \
+#ifdef __x86_64__
+#define _Unwind_Frames_Increment(exc, context, frames) \
+ { \
+ frames++; \
+ if (exc->exception_class != 0 \
+ && _Unwind_GetIP (context) != 0 \
+ && !_Unwind_IsSignalFrame (context)) \
+ { \
+ _Unwind_Word ssp = _get_ssp (); \
+ if (ssp != 0) \
+ { \
+ ssp += 8 * frames; \
+ _Unwind_Word ra = *(_Unwind_Word *) ssp; \
+ if (ra != _Unwind_GetIP (context)) \
+ return _URC_FATAL_PHASE2_ERROR; \
+ } \
+ } \
+ }
+#else
+#define _Unwind_Frames_Increment(exc, context, frames) \
if (_Unwind_IsSignalFrame (context)) \
do \
{ \
@@ -83,5 +112,19 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
} \
while (0); \
else \
- frames++;
+ { \
+ frames++; \
+ if (exc->exception_class != 0 \
+ && _Unwind_GetIP (context) != 0) \
+ { \
+ _Unwind_Word ssp = _get_ssp (); \
+ if (ssp != 0) \
+ { \
+ ssp += 4 * frames; \
+ _Unwind_Word ra = *(_Unwind_Word *) ssp; \
+ if (ra != _Unwind_GetIP (context)) \
+ return _URC_FATAL_PHASE2_ERROR; \
+ } \
+ } \
+ }
#endif
diff --git a/libgcc/unwind-generic.h b/libgcc/unwind-generic.h
index a87c9b3ccf6..bf721282d03 100644
--- a/libgcc/unwind-generic.h
+++ b/libgcc/unwind-generic.h
@@ -292,6 +292,6 @@ EXCEPTION_DISPOSITION _GCC_specific_handler (PEXCEPTION_RECORD, void *,
#define _Unwind_Frames_Extra(frames)
/* Increment frame count. */
-#define _Unwind_Frames_Increment(context, frames) frames++
+#define _Unwind_Frames_Increment(exc, context, frames) frames++
#endif /* unwind.h */
diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc
index 5efd8af1b15..a7111a7b3a8 100644
--- a/libgcc/unwind.inc
+++ b/libgcc/unwind.inc
@@ -73,7 +73,7 @@ _Unwind_RaiseException_Phase2(struct _Unwind_Exception *exc,
gcc_assert (!match_handler);
uw_update_context (context, &fs);
- _Unwind_Frames_Increment (context, frames);
+ _Unwind_Frames_Increment (exc, context, frames);
}
*frames_p = frames;
@@ -191,7 +191,7 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc,
/* Update cur_context to describe the same frame as fs, and discard
the previous context if necessary. */
uw_advance_context (context, &fs);
- _Unwind_Frames_Increment (context, frames);
+ _Unwind_Frames_Increment (exc, context, frames);
}
*frames_p = frames;
--
2.37.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86: Check corrupted return address when unwinding stack
2022-09-21 20:42 [PATCH] x86: Check corrupted return address when unwinding stack H.J. Lu
@ 2022-09-30 19:33 ` Jeff Law
2022-10-04 21:32 ` PING^1: " H.J. Lu
1 sibling, 0 replies; 4+ messages in thread
From: Jeff Law @ 2022-09-30 19:33 UTC (permalink / raw)
To: gcc-patches
On 9/21/22 14:42, H.J. Lu via Gcc-patches wrote:
> If shadow stack is enabled, when unwinding stack, we count how many stack
> frames we pop to reach the landing pad and adjust shadow stack by the same
> amount. When counting the stack frame, we compare the return address on
> normal stack against the return address on shadow stack. If they don't
> match, return _URC_FATAL_PHASE2_ERROR for the corrupted return address on
> normal stack. Don't check the return address for
>
> 1. Non-catchable exception where exception_class == 0. Process will be
> terminated.
> 2. Zero return address which marks the outermost stack frame.
> 3. Signal stack frame since kernel puts a restore token on shadow stack.
>
> * unwind-generic.h (_Unwind_Frames_Increment): Add the EXC
> argument.
> * unwind.inc (_Unwind_RaiseException_Phase2): Pass EXC to
> _Unwind_Frames_Increment.
> (_Unwind_ForcedUnwind_Phase2): Likewise.
> * config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
> Take the EXC argument. Return _URC_FATAL_PHASE2_ERROR if the
> return address on normal stack doesn't match the return address
> on shadow stack.
The generic bits are fine. While I'm aware of one other target that
_might_ need adjustment, it's an out-of-tree target and thus clearly not
your responsibility.
jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* PING^1: [PATCH] x86: Check corrupted return address when unwinding stack
2022-09-21 20:42 [PATCH] x86: Check corrupted return address when unwinding stack H.J. Lu
2022-09-30 19:33 ` Jeff Law
@ 2022-10-04 21:32 ` H.J. Lu
2022-10-17 7:24 ` Hongtao Liu
1 sibling, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2022-10-04 21:32 UTC (permalink / raw)
To: GCC Patches, Hongtao Liu; +Cc: Uros Bizjak
On Wed, Sep 21, 2022 at 1:42 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> If shadow stack is enabled, when unwinding stack, we count how many stack
> frames we pop to reach the landing pad and adjust shadow stack by the same
> amount. When counting the stack frame, we compare the return address on
> normal stack against the return address on shadow stack. If they don't
> match, return _URC_FATAL_PHASE2_ERROR for the corrupted return address on
> normal stack. Don't check the return address for
>
> 1. Non-catchable exception where exception_class == 0. Process will be
> terminated.
> 2. Zero return address which marks the outermost stack frame.
> 3. Signal stack frame since kernel puts a restore token on shadow stack.
>
> * unwind-generic.h (_Unwind_Frames_Increment): Add the EXC
> argument.
> * unwind.inc (_Unwind_RaiseException_Phase2): Pass EXC to
> _Unwind_Frames_Increment.
> (_Unwind_ForcedUnwind_Phase2): Likewise.
> * config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
> Take the EXC argument. Return _URC_FATAL_PHASE2_ERROR if the
> return address on normal stack doesn't match the return address
> on shadow stack.
> ---
> libgcc/config/i386/shadow-stack-unwind.h | 51 ++++++++++++++++++++++--
> libgcc/unwind-generic.h | 2 +-
> libgcc/unwind.inc | 4 +-
> 3 files changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h
> index 2b02682bdae..89d44165000 100644
> --- a/libgcc/config/i386/shadow-stack-unwind.h
> +++ b/libgcc/config/i386/shadow-stack-unwind.h
> @@ -54,10 +54,39 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
> aligned. If the original shadow stack is 8 byte aligned, we just
> need to pop 2 slots, one restore token, from shadow stack. Otherwise,
> we need to pop 3 slots, one restore token + 4 byte padding, from
> - shadow stack. */
> -#ifndef __x86_64__
> + shadow stack.
> +
> + When popping a stack frame, we compare the return address on normal
> + stack against the return address on shadow stack. If they don't match,
> + return _URC_FATAL_PHASE2_ERROR for the corrupted return address on
> + normal stack. Don't check the return address for
> + 1. Non-catchable exception where exception_class == 0. Process will
> + be terminated.
> + 2. Zero return address which marks the outermost stack frame.
> + 3. Signal stack frame since kernel puts a restore token on shadow
> + stack.
> + */
> #undef _Unwind_Frames_Increment
> -#define _Unwind_Frames_Increment(context, frames) \
> +#ifdef __x86_64__
> +#define _Unwind_Frames_Increment(exc, context, frames) \
> + { \
> + frames++; \
> + if (exc->exception_class != 0 \
> + && _Unwind_GetIP (context) != 0 \
> + && !_Unwind_IsSignalFrame (context)) \
> + { \
> + _Unwind_Word ssp = _get_ssp (); \
> + if (ssp != 0) \
> + { \
> + ssp += 8 * frames; \
> + _Unwind_Word ra = *(_Unwind_Word *) ssp; \
> + if (ra != _Unwind_GetIP (context)) \
> + return _URC_FATAL_PHASE2_ERROR; \
> + } \
> + } \
> + }
> +#else
> +#define _Unwind_Frames_Increment(exc, context, frames) \
> if (_Unwind_IsSignalFrame (context)) \
> do \
> { \
> @@ -83,5 +112,19 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
> } \
> while (0); \
> else \
> - frames++;
> + { \
> + frames++; \
> + if (exc->exception_class != 0 \
> + && _Unwind_GetIP (context) != 0) \
> + { \
> + _Unwind_Word ssp = _get_ssp (); \
> + if (ssp != 0) \
> + { \
> + ssp += 4 * frames; \
> + _Unwind_Word ra = *(_Unwind_Word *) ssp; \
> + if (ra != _Unwind_GetIP (context)) \
> + return _URC_FATAL_PHASE2_ERROR; \
> + } \
> + } \
> + }
> #endif
> diff --git a/libgcc/unwind-generic.h b/libgcc/unwind-generic.h
> index a87c9b3ccf6..bf721282d03 100644
> --- a/libgcc/unwind-generic.h
> +++ b/libgcc/unwind-generic.h
> @@ -292,6 +292,6 @@ EXCEPTION_DISPOSITION _GCC_specific_handler (PEXCEPTION_RECORD, void *,
> #define _Unwind_Frames_Extra(frames)
>
> /* Increment frame count. */
> -#define _Unwind_Frames_Increment(context, frames) frames++
> +#define _Unwind_Frames_Increment(exc, context, frames) frames++
>
> #endif /* unwind.h */
> diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc
> index 5efd8af1b15..a7111a7b3a8 100644
> --- a/libgcc/unwind.inc
> +++ b/libgcc/unwind.inc
> @@ -73,7 +73,7 @@ _Unwind_RaiseException_Phase2(struct _Unwind_Exception *exc,
> gcc_assert (!match_handler);
>
> uw_update_context (context, &fs);
> - _Unwind_Frames_Increment (context, frames);
> + _Unwind_Frames_Increment (exc, context, frames);
> }
>
> *frames_p = frames;
> @@ -191,7 +191,7 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc,
> /* Update cur_context to describe the same frame as fs, and discard
> the previous context if necessary. */
> uw_advance_context (context, &fs);
> - _Unwind_Frames_Increment (context, frames);
> + _Unwind_Frames_Increment (exc, context, frames);
> }
>
> *frames_p = frames;
> --
> 2.37.3
>
PING. Jeff has approved the generic changes.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PING^1: [PATCH] x86: Check corrupted return address when unwinding stack
2022-10-04 21:32 ` PING^1: " H.J. Lu
@ 2022-10-17 7:24 ` Hongtao Liu
0 siblings, 0 replies; 4+ messages in thread
From: Hongtao Liu @ 2022-10-17 7:24 UTC (permalink / raw)
To: H.J. Lu; +Cc: GCC Patches, Uros Bizjak
On Wed, Oct 5, 2022 at 5:33 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Sep 21, 2022 at 1:42 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > If shadow stack is enabled, when unwinding stack, we count how many stack
> > frames we pop to reach the landing pad and adjust shadow stack by the same
> > amount. When counting the stack frame, we compare the return address on
> > normal stack against the return address on shadow stack. If they don't
> > match, return _URC_FATAL_PHASE2_ERROR for the corrupted return address on
> > normal stack. Don't check the return address for
> >
> > 1. Non-catchable exception where exception_class == 0. Process will be
> > terminated.
> > 2. Zero return address which marks the outermost stack frame.
> > 3. Signal stack frame since kernel puts a restore token on shadow stack.
Ok.
> >
> > * unwind-generic.h (_Unwind_Frames_Increment): Add the EXC
> > argument.
> > * unwind.inc (_Unwind_RaiseException_Phase2): Pass EXC to
> > _Unwind_Frames_Increment.
> > (_Unwind_ForcedUnwind_Phase2): Likewise.
> > * config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
> > Take the EXC argument. Return _URC_FATAL_PHASE2_ERROR if the
> > return address on normal stack doesn't match the return address
> > on shadow stack.
> > ---
> > libgcc/config/i386/shadow-stack-unwind.h | 51 ++++++++++++++++++++++--
> > libgcc/unwind-generic.h | 2 +-
> > libgcc/unwind.inc | 4 +-
> > 3 files changed, 50 insertions(+), 7 deletions(-)
> >
> > diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h
> > index 2b02682bdae..89d44165000 100644
> > --- a/libgcc/config/i386/shadow-stack-unwind.h
> > +++ b/libgcc/config/i386/shadow-stack-unwind.h
> > @@ -54,10 +54,39 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
> > aligned. If the original shadow stack is 8 byte aligned, we just
> > need to pop 2 slots, one restore token, from shadow stack. Otherwise,
> > we need to pop 3 slots, one restore token + 4 byte padding, from
> > - shadow stack. */
> > -#ifndef __x86_64__
> > + shadow stack.
> > +
> > + When popping a stack frame, we compare the return address on normal
> > + stack against the return address on shadow stack. If they don't match,
> > + return _URC_FATAL_PHASE2_ERROR for the corrupted return address on
> > + normal stack. Don't check the return address for
> > + 1. Non-catchable exception where exception_class == 0. Process will
> > + be terminated.
> > + 2. Zero return address which marks the outermost stack frame.
> > + 3. Signal stack frame since kernel puts a restore token on shadow
> > + stack.
> > + */
> > #undef _Unwind_Frames_Increment
> > -#define _Unwind_Frames_Increment(context, frames) \
> > +#ifdef __x86_64__
> > +#define _Unwind_Frames_Increment(exc, context, frames) \
> > + { \
> > + frames++; \
> > + if (exc->exception_class != 0 \
> > + && _Unwind_GetIP (context) != 0 \
> > + && !_Unwind_IsSignalFrame (context)) \
> > + { \
> > + _Unwind_Word ssp = _get_ssp (); \
> > + if (ssp != 0) \
> > + { \
> > + ssp += 8 * frames; \
> > + _Unwind_Word ra = *(_Unwind_Word *) ssp; \
> > + if (ra != _Unwind_GetIP (context)) \
> > + return _URC_FATAL_PHASE2_ERROR; \
> > + } \
> > + } \
> > + }
> > +#else
> > +#define _Unwind_Frames_Increment(exc, context, frames) \
> > if (_Unwind_IsSignalFrame (context)) \
> > do \
> > { \
> > @@ -83,5 +112,19 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
> > } \
> > while (0); \
> > else \
> > - frames++;
> > + { \
> > + frames++; \
> > + if (exc->exception_class != 0 \
> > + && _Unwind_GetIP (context) != 0) \
> > + { \
> > + _Unwind_Word ssp = _get_ssp (); \
> > + if (ssp != 0) \
> > + { \
> > + ssp += 4 * frames; \
> > + _Unwind_Word ra = *(_Unwind_Word *) ssp; \
> > + if (ra != _Unwind_GetIP (context)) \
> > + return _URC_FATAL_PHASE2_ERROR; \
> > + } \
> > + } \
> > + }
> > #endif
> > diff --git a/libgcc/unwind-generic.h b/libgcc/unwind-generic.h
> > index a87c9b3ccf6..bf721282d03 100644
> > --- a/libgcc/unwind-generic.h
> > +++ b/libgcc/unwind-generic.h
> > @@ -292,6 +292,6 @@ EXCEPTION_DISPOSITION _GCC_specific_handler (PEXCEPTION_RECORD, void *,
> > #define _Unwind_Frames_Extra(frames)
> >
> > /* Increment frame count. */
> > -#define _Unwind_Frames_Increment(context, frames) frames++
> > +#define _Unwind_Frames_Increment(exc, context, frames) frames++
> >
> > #endif /* unwind.h */
> > diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc
> > index 5efd8af1b15..a7111a7b3a8 100644
> > --- a/libgcc/unwind.inc
> > +++ b/libgcc/unwind.inc
> > @@ -73,7 +73,7 @@ _Unwind_RaiseException_Phase2(struct _Unwind_Exception *exc,
> > gcc_assert (!match_handler);
> >
> > uw_update_context (context, &fs);
> > - _Unwind_Frames_Increment (context, frames);
> > + _Unwind_Frames_Increment (exc, context, frames);
> > }
> >
> > *frames_p = frames;
> > @@ -191,7 +191,7 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc,
> > /* Update cur_context to describe the same frame as fs, and discard
> > the previous context if necessary. */
> > uw_advance_context (context, &fs);
> > - _Unwind_Frames_Increment (context, frames);
> > + _Unwind_Frames_Increment (exc, context, frames);
> > }
> >
> > *frames_p = frames;
> > --
> > 2.37.3
> >
>
> PING. Jeff has approved the generic changes.
>
> Thanks.
>
> --
> H.J.
--
BR,
Hongtao
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-17 7:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21 20:42 [PATCH] x86: Check corrupted return address when unwinding stack H.J. Lu
2022-09-30 19:33 ` Jeff Law
2022-10-04 21:32 ` PING^1: " H.J. Lu
2022-10-17 7:24 ` Hongtao Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).