x86: SROP mitigation: implement signal canaries
PATCH 2/4 x86: SROP mitigation: implement signal canaries
This patch implements signal canaries for x86-64, x86-32 and x32.
--- arch/x86/Kconfig | 1 + arch/x86/ia32/ia32_signal.c | 27 +++++++++++++++++++++++++++ arch/x86/include/asm/sigframe.h | 12 ++++++++++++ arch/x86/kernel/signal.c | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+)
arch/x86/Kconfig +1 -0
@@ -128,6 +128,7 @@ config X86 select HAVE_DEBUG_STACKOVERFLOW select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64 select HAVE_CC_STACKPROTECTOR + select HAVE_SIGNAL_CANARY select GENERIC_CPU_AUTOPROBE select HAVE_ARCH_AUDITSYSCALL
arch/x86/ia32/ia32_signal.c +27 -0
@@ -212,9 +212,18 @@ asmlinkage long sys32_sigreturn(void)struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user *)(regs->sp-8 );sigset_t set;unsigned int ax; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif if (!access_ok (VERIFY_READ , frame,sizeof (*frame))) goto badframe; + +#ifdef CONFIG_SIGNAL_CANARY +if (__get_user (canary, &frame->canary) || (canary != current->signal_canary)) + goto badframe; +#endif + if (__get_user(set.sig[0], &frame->sc.oldmask) || (_COMPAT_NSIG_WORDS > 1 && __copy_from_user((((char *) &set.sig) + 4),
@@ -239,11 +248,20 @@ asmlinkage long sys32_rt_sigreturn(void)struct rt_sigframe_ia32 __user *frame;sigset_t set;unsigned int ax; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif frame = (struct rt_sigframe_ia32 __user *)(regs->sp -4 );if (!access_ok (VERIFY_READ , frame,sizeof (*frame))) goto badframe; + +#ifdef CONFIG_SIGNAL_CANARY +if (__get_user (canary, &frame->canary) || (canary != current->signal_canary)) + goto badframe; +#endif +if (__copy_from_user (&set, &frame->uc.uc_sigmask,sizeof (set))) goto badframe;
@@ -378,6 +396,11 @@ int ia32_setup_frame(int sig, struct ksignal *ksig,return -EFAULT ; } +#ifdef CONFIG_SIGNAL_CANARY +if (__put_user (current->signal_canary, &frame->canary)) +return -EFAULT ; +#endif +if (ksig->ka.sa.sa_flags &SA_RESTORER ) { restorer = ksig->ka.sa.sa_restorer; }else {
@@ -459,6 +482,10 @@ int ia32_setup_rt_frame(int sig, struct ksignal *ksig,put_user_ex (0 , &frame->uc.uc_link);compat_save_altstack_ex (&frame->uc.uc_stack, regs->sp); +#ifdef CONFIG_SIGNAL_CANARY +put_user_ex (current->signal_canary, &frame->canary); +#endif +if (ksig->ka.sa.sa_flags &SA_RESTORER ) restorer = ksig->ka.sa.sa_restorer;else
arch/x86/include/asm/sigframe.h +12 -0
@@ -39,6 +39,9 @@ struct sigframe_ia32 {unsigned long extramask[_NSIG_WORDS-1 ];#endif /* CONFIG_IA32_EMULATION */ char retcode[8 ]; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif /* fp state follows here */ };
@@ -54,6 +57,9 @@ struct rt_sigframe_ia32 {#endif /* CONFIG_IA32_EMULATION */ struct ucontext_ia32 uc;char retcode[8 ]; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif /* fp state follows here */ };#endif /* defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION) */
@@ -64,6 +70,9 @@ struct rt_sigframe {char __user *pretcode;struct ucontext uc;struct siginfo info; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif /* fp state follows here */ };
@@ -73,6 +82,9 @@ struct rt_sigframe_x32 {u64 pretcode;struct ucontext_x32 uc;compat_siginfo_t info; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif /* fp state follows here */ };
arch/x86/kernel/signal.c +41 -0
98 lines not on this page · raw message
Signature · 1 line
1.9.1
arch/x86/ia32/ia32_signal.c asmlinkage long sys32_sigreturn(void) · line 217
Quoted 12 lines · Erik Bosman
arch/x86/ia32/ia32_signal.c +3
-0@@ -212,9 +212,18 @@ asmlinkage long sys32_sigreturn(void)struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user *)(regs->sp-8 );sigset_t set;unsigned int ax; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif
Don't you completely break the ABI here? I'm sure there are programs out there who hard code the offset into the FP state.
I think you either need to put it at the total end or somewhere currently unused
Besides that I would remove the CONFIG_* once it works and just do it unconditionally.
-Andi
Signature · 1 line
[email protected] -- Speaking for myself only
Quoted 18 lines · Andi Kleen
arch/x86/ia32/ia32_signal.c asmlinkage long sys32_sigreturn(void) · line 217
Quoted 12 lines · Erik Bosman
arch/x86/ia32/ia32_signal.c +3
-0@@ -212,9 +212,18 @@ asmlinkage long sys32_sigreturn(void)struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user *)(regs->sp-8 );sigset_t set;unsigned int ax; +#ifdef CONFIG_SIGNAL_CANARY +u32 canary; +#endif Don't you completely break the ABI here? I'm sure there are programs out there who hard code the offset into the FP state.
Quoted 3 lines
I think you either need to put it at the total end or somewhere currently unused
Hrmz.. FP state is aligned on a 64 byte boundary, the signal frame (separately) on a 16 byte-sizeof(long) boundary. But it looks like that for 32 and 64 bit rt_sigreturn this means no padding :-/
It looks like I'll have to put the canary beyond the fp state. :-( I had hoped to avoid pointer arithmetic. :-/
Erik
Quoted 14 lines
Besides that I would remove the CONFIG_* once it works and just do it unconditionally.
-Andi
Signature · 6 lines
[email protected] -- Speaking for myself only -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Quoted 3 lines
Hrmz.. FP state is aligned on a 64 byte boundary, the signal frame (separately) on a 16 byte-sizeof(long) boundary. But it looks like that for 32 and 64 bit
rt_sigreturnthis means no padding :-/
There may be some unused bytes in the existing structures, perhaps not continuous.
-Andi