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 <abhi...@cs.utah.edu>
>> Signed-off-by: Scott Bauer <sba...@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
