On Mon, Feb 27, 2023 at 02:29:48PM -0800, Rick Edgecombe wrote:
> From: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
>
> When a signal is handled normally the context is pushed to the stack
s/normally //
> before handling it. For shadow stacks, since the shadow stack only track's
"tracks"
> return addresses, there isn't any state that needs to be pushed. However,
> there are still a few things that need to be done. These things are
> userspace visible and which will be kernel ABI for shadow stacks.
"visible to userspace"
s/which //
> One is to make sure the restorer address is written to shadow stack, since
> the signal handler (if not changing ucontext) returns to the restorer, and
> the restorer calls sigreturn. So add the restorer on the shadow stack
> before handling the signal, so there is not a conflict when the signal
> handler returns to the restorer.
>
> The other thing to do is to place some type of checkable token on the
> thread's shadow stack before handling the signal and check it during
> sigreturn. This is an extra layer of protection to hamper attackers
> calling sigreturn manually as in SROP-like attacks.
>
> For this token we can use the shadow stack data format defined earlier.
^^^
Please use passive voice in your commit message: no "we" or "I", etc.
> Have the data pushed be the previous SSP. In the future the sigreturn
> might want to return back to a different stack. Storing the SSP (instead
> of a restore offset or something) allows for future functionality that
> may want to restore to a different stack.
>
> So, when handling a signal push
> - the SSP pointing in the shadow stack data format
> - the restorer address below the restore token.
>
> In sigreturn, verify SSP is stored in the data format and pop the shadow
> stack.
...
> diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
> index 13c02747386f..40f0a55762a9 100644
> --- a/arch/x86/kernel/shstk.c
> +++ b/arch/x86/kernel/shstk.c
> @@ -232,6 +232,104 @@ static int get_shstk_data(unsigned long *data, unsigned long __user *addr)
> return 0;
> }
>
> +static int shstk_push_sigframe(unsigned long *ssp)
> +{
> + unsigned long target_ssp = *ssp;
> +
> + /* Token must be aligned */
> + if (!IS_ALIGNED(*ssp, 8))
> + return -EINVAL;
> +
> + if (!IS_ALIGNED(target_ssp, 8))
> + return -EINVAL;
Those two statements are identical AFAICT.
> + *ssp -= SS_FRAME_SIZE;
> + if (put_shstk_data((void *__user)*ssp, target_ssp))
> + return -EFAULT;
> +
> + return 0;
> +}
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
