Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1353397 > unrolled thread

[PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies Scott Bauer <sbauer@eng.utah.edu> - 2016-03-08 21:50 +0100
    Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for  signal cookies Andy Lutomirski <luto@amacapital.net> - 2016-03-08 22:00 +0100
      Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for  signal cookies Andy Lutomirski <luto@amacapital.net> - 2016-03-08 23:00 +0100
        Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for  signal cookies Scotty Bauer <sbauer@eng.utah.edu> - 2016-03-08 23:10 +0100
        Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for  signal cookies Scotty Bauer <sbauer@eng.utah.edu> - 2016-03-09 23:10 +0100
      Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for  signal cookies Scotty Bauer <sbauer@eng.utah.edu> - 2016-03-08 23:00 +0100
    Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code  for signal cookies kbuild test robot <lkp@intel.com> - 2016-03-08 22:10 +0100
    Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code  for signal cookies Ingo Molnar <mingo@kernel.org> - 2016-03-09 09:40 +0100
      Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for  signal cookies Scotty Bauer <sbauer@eng.utah.edu> - 2016-03-09 23:10 +0100
        Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code  for signal cookies Jonathan Corbet <corbet@lwn.net> - 2016-03-09 23:30 +0100
          Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code  for signal cookies Ingo Molnar <mingo@kernel.org> - 2016-03-10 10:50 +0100

#1353397 — [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

This patch adds a per-process secret to the task struct which
will be used during signal delivery and during a sigreturn.
Also, logic is added in signal.c to generate, place, extract,
clear and verify the signal cookie.

Cc: Abhiram Balasubramanian <abhiram@cs.utah.edu>
Signed-off-by: Scott Bauer <sbauer@eng.utah.edu>
---
 fs/exec.c              |  3 +++
 include/linux/sched.h  |  7 +++++++
 include/linux/signal.h |  2 ++
 kernel/signal.c        | 40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index dcd4ac7..3de0a32 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -56,6 +56,7 @@
 #include <linux/pipe_fs_i.h>
 #include <linux/oom.h>
 #include <linux/compat.h>
+#include <linux/random.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -1135,6 +1136,8 @@ void setup_new_exec(struct linux_binprm * bprm)
 	/* This is the point of no return */
 	current->sas_ss_sp = current->sas_ss_size = 0;
 
+	get_random_bytes(&current->sig_cookie, sizeof(current->sig_cookie));
+
 	if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
 		set_dumpable(current->mm, SUID_DUMP_USER);
 	else
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a10494a..556162f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1497,6 +1497,13 @@ struct task_struct {
 	unsigned long stack_canary;
 #endif
 	/*
+	 * Canary value for signal frames placed on user stack.
+	 * This helps mitigate "Signal Return oriented program"
+	 * exploits in userland.
+	 */
+	unsigned long sig_cookie;
+
+	/*
 	 * pointers to (original) parent process, youngest child, younger sibling,
 	 * older sibling, respectively.  (p->father can be replaced with
 	 * p->real_parent->pid)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 92557bb..fae0618 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -280,6 +280,8 @@ extern int get_signal(struct ksignal *ksig);
 extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
 extern void exit_signals(struct task_struct *tsk);
 extern void kernel_sigaction(int, __sighandler_t);
+extern int set_sigcookie(unsigned long __user *location);
+extern int verify_clear_sigcookie(unsigned long __user *sig_cookie_ptr);
 
 static inline void allow_signal(int sig)
 {
diff --git a/kernel/signal.c b/kernel/signal.c
index 0508544..00e4a16 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2430,6 +2430,46 @@ out:
 	}
 }
 
+static unsigned long gen_sigcookie(unsigned long __user *location)
+{
+
+	unsigned long sig_cookie;
+	sig_cookie = (unsigned long) location ^ current->sig_cookie;
+
+	return sig_cookie;
+}
+
+int set_sigcookie(unsigned long __user *location)
+{
+
+	unsigned long sig_cookie = gen_sigcookie(location);
+
+	return put_user(sig_cookie, location);
+}
+
+int verify_clear_sigcookie(unsigned long __user *sig_cookie_ptr)
+{
+	unsigned long user_cookie;
+	unsigned long calculated_cookie;
+
+	if (get_user(user_cookie, sig_cookie_ptr))
+		return 1;
+
+	calculated_cookie = gen_sigcookie(sig_cookie_ptr);
+
+	if (user_cookie != calculated_cookie) {
+		pr_warn("Signal protector does not match what kernel set it to"\
+			". Possible exploit attempt or buggy program!\n");
+		return 1;
+
+	}
+
+	user_cookie = 0;
+	return put_user(user_cookie, sig_cookie_ptr)
+}
+
+EXPORT_SYMBOL(verify_clear_sigcookie);
+EXPORT_SYMBOL(set_sigcookie);
 EXPORT_SYMBOL(recalc_sigpending);
 EXPORT_SYMBOL_GPL(dequeue_signal);
 EXPORT_SYMBOL(flush_signals);
-- 
1.9.1

[toc] | [next] | [standalone]

#1353403 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

On Tue, Mar 8, 2016 at 12:47 PM, Scott Bauer <sbauer@eng.utah.edu> wrote:
> This patch adds a per-process secret to the task struct which
> will be used during signal delivery and during a sigreturn.
> Also, logic is added in signal.c to generate, place, extract,
> clear and verify the signal cookie.
>

Potentially silly question: it's been a while since I read the SROP
paper, but would the technique be effectively mitigated if sigreturn
were to zero out the whole signal frame before returning to user mode?

[toc] | [prev] | [next] | [standalone]

#1353432 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

On Tue, Mar 8, 2016 at 1:49 PM, Scotty Bauer <sbauer@eng.utah.edu> wrote:
>
>
> On 03/08/2016 01:58 PM, Andy Lutomirski wrote:
>> On Tue, Mar 8, 2016 at 12:47 PM, Scott Bauer <sbauer@eng.utah.edu> wrote:
>>> This patch adds a per-process secret to the task struct which
>>> will be used during signal delivery and during a sigreturn.
>>> Also, logic is added in signal.c to generate, place, extract,
>>> clear and verify the signal cookie.
>>>
>>
>> Potentially silly question: it's been a while since I read the SROP
>> paper, but would the technique be effectively mitigated if sigreturn
>> were to zero out the whole signal frame before returning to user mode?
>>
>
> I don't know if I fully understand your question, but I'll respond anyway.
>
> SROP is possible because the kernel doesn't know whether or not the
> incoming sigreturn syscall is in response from a legitimate signal that
> the kernel had previously delivered and the program handled. So essentially
> these patches are an attempt to give the kernel a way to verify whether or
> not the the incoming sigreturn is a valid response or a exploit trying to
> hijack control of the user program.
>

I got that part, but I thought that the interesting SROP bit was using
sigreturn to return back to a frame where you could just repeat the
sigreturn a bunch of times to compute things and do other evil.  I'm
wondering whether zeroing the whole frame would make SROP much less
interesting to an attacker.

--Andy

[toc] | [prev] | [next] | [standalone]

#1353441 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

On 03/08/2016 02:57 PM, Andy Lutomirski wrote:
> On Tue, Mar 8, 2016 at 1:49 PM, Scotty Bauer <sbauer@eng.utah.edu> wrote:
>>
>>
>> On 03/08/2016 01:58 PM, Andy Lutomirski wrote:
>>> On Tue, Mar 8, 2016 at 12:47 PM, Scott Bauer <sbauer@eng.utah.edu> wrote:
>>>> This patch adds a per-process secret to the task struct which
>>>> will be used during signal delivery and during a sigreturn.
>>>> Also, logic is added in signal.c to generate, place, extract,
>>>> clear and verify the signal cookie.
>>>>
>>>
>>> Potentially silly question: it's been a while since I read the SROP
>>> paper, but would the technique be effectively mitigated if sigreturn
>>> were to zero out the whole signal frame before returning to user mode?
>>>
>>
>> I don't know if I fully understand your question, but I'll respond anyway.
>>
>> SROP is possible because the kernel doesn't know whether or not the
>> incoming sigreturn syscall is in response from a legitimate signal that
>> the kernel had previously delivered and the program handled. So essentially
>> these patches are an attempt to give the kernel a way to verify whether or
>> not the the incoming sigreturn is a valid response or a exploit trying to
>> hijack control of the user program.
>>
> 
> I got that part, but I thought that the interesting SROP bit was using
> sigreturn to return back to a frame where you could just repeat the
> sigreturn a bunch of times to compute things and do other evil.  I'm
> wondering whether zeroing the whole frame would make SROP much less
> interesting to an attacker.
> 
> --Andy
> 

Ah, I see now. I believe that would work for subsequent sigreturns but not
the first.

The paper did talk about actually ROP'ing using sigreturns but I never really
liked that idea. I think they missed the obvious reason an attacker would use
SROP. The reason an attacker would use is it gives you an extremely easy
way to get values into registers.. you just set them to what you want them to be
then sigret. Previously an attacker would have to find gadgets and ROP around until
the registers are in a good enough state to do what ever they want. 

I mostly designed the patches with that in mind instead of actually ROPing with sigret.

I can certainly add zeroing the stack frame, but not sure if there would be a performance
regression in doing so or if it's really relevant if we keep the cookie.

 

[toc] | [prev] | [next] | [standalone]

#1354516 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

On 03/08/2016 02:57 PM, Andy Lutomirski wrote:
> On Tue, Mar 8, 2016 at 1:49 PM, Scotty Bauer <sbauer@eng.utah.edu> wrote:
>>
>>
>> On 03/08/2016 01:58 PM, Andy Lutomirski wrote:
>>> On Tue, Mar 8, 2016 at 12:47 PM, Scott Bauer <sbauer@eng.utah.edu> wrote:
>>>> This patch adds a per-process secret to the task struct which
>>>> will be used during signal delivery and during a sigreturn.
>>>> Also, logic is added in signal.c to generate, place, extract,
>>>> clear and verify the signal cookie.
>>>>
>>>
>>> Potentially silly question: it's been a while since I read the SROP
>>> paper, but would the technique be effectively mitigated if sigreturn
>>> were to zero out the whole signal frame before returning to user mode?
>>>
>>
>> I don't know if I fully understand your question, but I'll respond anyway.
>>
>> SROP is possible because the kernel doesn't know whether or not the
>> incoming sigreturn syscall is in response from a legitimate signal that
>> the kernel had previously delivered and the program handled. So essentially
>> these patches are an attempt to give the kernel a way to verify whether or
>> not the the incoming sigreturn is a valid response or a exploit trying to
>> hijack control of the user program.
>>
> 
> I got that part, but I thought that the interesting SROP bit was using
> sigreturn to return back to a frame where you could just repeat the
> sigreturn a bunch of times to compute things and do other evil.  I'm
> wondering whether zeroing the whole frame would make SROP much less
> interesting to an attacker.
> 
> --Andy
> 

I've been thinking about this a little bit more. I don't think zeroing the frame
is a proper mitigation. If an attacker has the ability to write a lot of data to
the stack they could simply create a new fake signal frame above the current frame.
In this scenario the kernel would zero the current frame then return somewhere attacker
controlled, where the attackers payload would then use the next signal frame above
the zero'd frame.

So while this zeroing would solve a stricter case where an attacker has to keep reusing
the same frame over and over, perhaps to avoid overwriting a stack cookie, It doesn't solve
every case. 

Thanks for the good ideas.

[toc] | [prev] | [next] | [standalone]

#1353436 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

On 03/08/2016 01:58 PM, Andy Lutomirski wrote:
> On Tue, Mar 8, 2016 at 12:47 PM, Scott Bauer <sbauer@eng.utah.edu> wrote:
>> This patch adds a per-process secret to the task struct which
>> will be used during signal delivery and during a sigreturn.
>> Also, logic is added in signal.c to generate, place, extract,
>> clear and verify the signal cookie.
>>
> 
> Potentially silly question: it's been a while since I read the SROP
> paper, but would the technique be effectively mitigated if sigreturn
> were to zero out the whole signal frame before returning to user mode?
> 

I don't know if I fully understand your question, but I'll respond anyway.

SROP is possible because the kernel doesn't know whether or not the
incoming sigreturn syscall is in response from a legitimate signal that
the kernel had previously delivered and the program handled. So essentially
these patches are an attempt to give the kernel a way to verify whether or 
not the the incoming sigreturn is a valid response or a exploit trying to
hijack control of the user program.

So no, zeroing out the frame wouldn't do much because if I understand your
question correctly once we call sigreturn the kernel is going to hand off
control to wherever the sigframe tells it to so I don't think zeroing would
do much.

The reason why I zero out the cookie is so if there is a stack leak bug or 
something along those lines an attacker couldnt leak the cookie and try and
derive what the per-process kernel secret is.

Hope that clarifies!

[toc] | [prev] | [next] | [standalone]

#1353405 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

[Multipart message — attachments visible in raw view] — view raw

Hi Scott,

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.5-rc7]
[cannot apply to next-20160308]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Scott-Bauer/SROP-Mitigation-Architecture-independent-code-for-signal-cookies/20160309-045202
config: i386-tinyconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Scott-Bauer/SROP-Mitigation-Architecture-independent-code-for-signal-cookies/20160309-045202 HEAD 52ff6d1c09bbf7b33f786ccaa3de5521002a92fa builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   kernel/signal.c: In function 'verify_clear_sigcookie':
>> kernel/signal.c:2469:1: error: expected ';' before '}' token
    }
    ^

vim +2469 kernel/signal.c

  2463			return 1;
  2464	
  2465		}
  2466	
  2467		user_cookie = 0;
  2468		return put_user(user_cookie, sig_cookie_ptr)
> 2469	}
  2470	
  2471	EXPORT_SYMBOL(verify_clear_sigcookie);
  2472	EXPORT_SYMBOL(set_sigcookie);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1353900 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

* Scott Bauer <sbauer@eng.utah.edu> wrote:

> This patch adds a per-process secret to the task struct which
> will be used during signal delivery and during a sigreturn.
> Also, logic is added in signal.c to generate, place, extract,
> clear and verify the signal cookie.

>  	/*
> +	 * Canary value for signal frames placed on user stack.
> +	 * This helps mitigate "Signal Return oriented program"
> +	 * exploits in userland.
> +	 */
> +	unsigned long sig_cookie;

Could you please add a high level description in Documentation
that explains the attack and the way how this mitigation code
prevents that kind of attack?

Also, the first changelogs should contain more high level
description as well. For example, what does the 'verification'
of the signal cookie mean, and how does it prevent an SROP
attempt?

All of these patches seem to assume that people reading this code
know what SROP is and how we defend against it - that is not so.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1354505 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

On 03/09/2016 01:32 AM, Ingo Molnar wrote:
> 
> * Scott Bauer <sbauer@eng.utah.edu> wrote:
> 
>> This patch adds a per-process secret to the task struct which
>> will be used during signal delivery and during a sigreturn.
>> Also, logic is added in signal.c to generate, place, extract,
>> clear and verify the signal cookie.
> 
>>  	/*
>> +	 * Canary value for signal frames placed on user stack.
>> +	 * This helps mitigate "Signal Return oriented program"
>> +	 * exploits in userland.
>> +	 */
>> +	unsigned long sig_cookie;
> 
> Could you please add a high level description in Documentation
> that explains the attack and the way how this mitigation code
> prevents that kind of attack?
> 
> Also, the first changelogs should contain more high level
> description as well. For example, what does the 'verification'
> of the signal cookie mean, and how does it prevent an SROP
> attempt?
> 
> All of these patches seem to assume that people reading this code
> know what SROP is and how we defend against it - that is not so.
> 
> Thanks,
> 
> 	Ingo
> 


I'm going to submit v4 to fix some nits where I'll include the explanation
and a change log, I apologize for not doing that here. In the meantime if
you don't mind visiting a link I included a brief explanation on previous
versions of the patch set.


https://lkml.org/lkml/2016/2/6/166

Thanks

[toc] | [prev] | [next] | [standalone]

#1354529 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

On Wed, 9 Mar 2016 15:07:07 -0700
Scotty Bauer <sbauer@eng.utah.edu> wrote:

> On 03/09/2016 01:32 AM, Ingo Molnar wrote:
> > 
> > Could you please add a high level description in Documentation
> > that explains the attack and the way how this mitigation code
> > prevents that kind of attack?
> > 
> > Also, the first changelogs should contain more high level
> > description as well. For example, what does the 'verification'
> > of the signal cookie mean, and how does it prevent an SROP
> > attempt?
> > 
> > All of these patches seem to assume that people reading this code
> > know what SROP is and how we defend against it - that is not so.
> 
> I'm going to submit v4 to fix some nits where I'll include the explanation
> and a change log, I apologize for not doing that here. In the meantime if
> you don't mind visiting a link I included a brief explanation on previous
> versions of the patch set.
> 
> https://lkml.org/lkml/2016/2/6/166

The curious might also find background information in my article about this
patch set:

      https://lwn.net/Articles/676803/

(The information still belongs with the patch posting, of course...)

jon

[toc] | [prev] | [next] | [standalone]

#1354960 — Re: [PATCH v3 1/3] SROP Mitigation: Architecture independent code for signal cookies

* Jonathan Corbet <corbet@lwn.net> wrote:

> On Wed, 9 Mar 2016 15:07:07 -0700
> Scotty Bauer <sbauer@eng.utah.edu> wrote:
> 
> > On 03/09/2016 01:32 AM, Ingo Molnar wrote:
> > > 
> > > Could you please add a high level description in Documentation
> > > that explains the attack and the way how this mitigation code
> > > prevents that kind of attack?
> > > 
> > > Also, the first changelogs should contain more high level
> > > description as well. For example, what does the 'verification'
> > > of the signal cookie mean, and how does it prevent an SROP
> > > attempt?
> > > 
> > > All of these patches seem to assume that people reading this code
> > > know what SROP is and how we defend against it - that is not so.
> > 
> > I'm going to submit v4 to fix some nits where I'll include the explanation
> > and a change log, I apologize for not doing that here. In the meantime if
> > you don't mind visiting a link I included a brief explanation on previous
> > versions of the patch set.
> > 
> > https://lkml.org/lkml/2016/2/6/166
> 
> The curious might also find background information in my article about this
> patch set:
> 
>       https://lwn.net/Articles/676803/

Scott, mind including a prominent link to the (excellent!) LWN.net article in the 
changelog/documentation as well?

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web