lkml.org 
[lkml]   [2016]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCHv2 2/2] x86: SROP mitigation: implement signal cookies
    From
    Date


    On 02/06/2016 11:35 PM, Mika Penttilä wrote:
    > 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@cs.utah.edu>
    >> Signed-off-by: Scott Bauer <sbauer@eng.utah.edu>
    >> ---
    >> 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
    >

    Nice catch, thank you. I'm surprised I haven't hit this issue yet.
    I've been running these patches for 3 weeks on 2 different machines and
    haven't had an issue. I'll submit v3 with this fix a bit later, I want
    to see if anyone else has stuff to fix.

    Thanks again

    \
     
     \ /
      Last update: 2016-02-07 09:41    [W:3.990 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site