Re: [PATCHv2 2/2] x86: SROP mitigation: implement signal cookies

From: Mika PenttilÃ
Date: Sun Feb 07 2016 - 01:36:16 EST


Hi,


On 07.02.2016 01:39, Scott Bauer wrote:
> This patch adds SROP mitigation logic to the x86 signal delivery
> and sigreturn code. The cookie is placed in the unused alignment
> space above the saved FP state, if it exists. If there is no FP
> state to save then the cookie is placed in the alignment space above
> the sigframe.
>
> Cc: Abhiram Balasubramanian <abhiram@xxxxxxxxxxx>
> Signed-off-by: Scott Bauer <sbauer@xxxxxxxxxxxx>
> ---
> arch/x86/ia32/ia32_signal.c | 63 +++++++++++++++++++++++++---
> arch/x86/include/asm/fpu/signal.h | 1 +
> arch/x86/include/asm/sighandling.h | 5 ++-
> arch/x86/kernel/fpu/signal.c | 10 +++++
> arch/x86/kernel/signal.c | 86 +++++++++++++++++++++++++++++++++-----
> 5 files changed, 146 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
> index 0552884..2751f47 100644
> --- a/arch/x86/ia32/ia32_signal.c
> +++ b/arch/x86/ia32/ia32_signal.c
> @@ -68,7 +68,8 @@
> }
>
> static int ia32_restore_sigcontext(struct pt_regs *regs,
> - struct sigcontext_32 __user *sc)
> + struct sigcontext_32 __user *sc,
> + void __user **user_cookie)
> {
> unsigned int tmpflags, err = 0;
> void __user *buf;
> @@ -105,6 +106,16 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
> buf = compat_ptr(tmp);
> } get_user_catch(err);
>
> + /*
> + * If there is fp state get cookie from the top of the fp state,
> + * else get it from the top of the sig frame.
> + */
> +
> + if (tmp != 0)
> + *user_cookie = compat_ptr(tmp + fpu__getsize(1));
> + else
> + *user_cookie = NULL;
> +
> err |= fpu__restore_sig(buf, 1);
>
> force_iret();
> @@ -117,6 +128,7 @@ asmlinkage long sys32_sigreturn(void)
> struct pt_regs *regs = current_pt_regs();
> struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user *)(regs->sp-8);
> sigset_t set;
> + void __user *user_cookie;
>
> if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
> goto badframe;
> @@ -129,8 +141,15 @@ asmlinkage long sys32_sigreturn(void)
>
> set_current_blocked(&set);
>
> - if (ia32_restore_sigcontext(regs, &frame->sc))
> + if (ia32_restore_sigcontext(regs, &frame->sc, &user_cookie))
> + goto badframe;
> +
> + if (user_cookie == NULL)
> + user_cookie = compat_ptr((regs->sp - 8) + sizeof(*frame));
> +
> + if (verify_clear_sigcookie(user_cookie))
> goto badframe;
> +
> return regs->ax;
>
> badframe:
> @@ -142,6 +161,7 @@ asmlinkage long sys32_rt_sigreturn(void)
> {
> struct pt_regs *regs = current_pt_regs();
> struct rt_sigframe_ia32 __user *frame;
> + void __user *user_cookie;
> sigset_t set;
>
> frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);
> @@ -153,7 +173,13 @@ asmlinkage long sys32_rt_sigreturn(void)
>
> set_current_blocked(&set);
>
> - if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext))
> + if (ia32_restore_sigcontext(regs, &frame->uc.uc_mcontext, &user_cookie))
> + goto badframe;
> +
> + if (user_cookie == NULL)
> + user_cookie = (void __user *)((regs->sp - 4) + sizeof(*frame));
regs->sp is already restored so you should use frame instead.

--Mika