[RFC][PATCH] selinux: apply execstack check on thread stacks

[RFC][PATCH] selinux: apply execstack check on thread stacks

[Stephen Smalley]

The execstack check was only being applied on the main
process stack.  Thread stacks allocated via mmap were
only subject to the execmem permission check.  Augment
the check to apply to the current thread stack as well.
Note that this does NOT prevent making a different thread's
stack executable.

Suggested-by: Nick Kralevich <nnk@google.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 security/selinux/hooks.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a9ca5ee..0271be4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3465,8 +3465,9 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
 		    vma->vm_end <= vma->vm_mm->brk) {
 			rc = cred_has_perm(cred, cred, PROCESS__EXECHEAP);
 		} else if (!vma->vm_file &&
-			   vma->vm_start <= vma->vm_mm->start_stack &&
-			   vma->vm_end >= vma->vm_mm->start_stack) {
+			   ((vma->vm_start <= vma->vm_mm->start_stack &&
+			     vma->vm_end >= vma->vm_mm->start_stack) ||
+			    vma_is_stack_for_task(vma, current))) {
 			rc = current_has_perm(current, PROCESS__EXECSTACK);
 		} else if (vma->vm_file && vma->anon_vma) {
 			/*
-- 
2.8.0

[RFC][PATCH] selinux-testsuite: Add test for execstack on thread stack

[Stephen Smalley]

Test execstack permission checking for thread stacks.
This depends on the corresponding kernel patch to apply
the check for thread stacks in addition to the main process
stack.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 tests/mmap/Makefile                |  2 ++
 tests/mmap/mprotect_stack_thread.c | 33 +++++++++++++++++++++++++++++++++
 tests/mmap/test                    |  8 +++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 tests/mmap/mprotect_stack_thread.c

diff --git a/tests/mmap/Makefile b/tests/mmap/Makefile
index f2f486c..e330f3e 100644
--- a/tests/mmap/Makefile
+++ b/tests/mmap/Makefile
@@ -1,5 +1,7 @@
 TARGETS=$(patsubst %.c,%,$(wildcard *.c))
 
+LDLIBS += -lpthread
+
 all: $(TARGETS)
 
 clean:
diff --git a/tests/mmap/mprotect_stack_thread.c b/tests/mmap/mprotect_stack_thread.c
new file mode 100644
index 0000000..457b294
--- /dev/null
+++ b/tests/mmap/mprotect_stack_thread.c
@@ -0,0 +1,33 @@
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <pthread.h>
+
+static void *test_thread(void *p)
+{
+	char buf[4096];
+	int rc;
+	void *ptr;
+	long pagesize = sysconf(_SC_PAGESIZE);
+
+	ptr = (void *) (((unsigned long) buf) & ~(pagesize - 1));
+
+	rc = mprotect(ptr, pagesize, PROT_READ | PROT_WRITE | PROT_EXEC);
+	if (rc < 0) {
+		perror("mprotect");
+		exit(1);
+	}
+	return NULL;
+}
+
+int main(void)
+{
+	pthread_t thread;
+
+	pthread_create(&thread, NULL, test_thread, NULL);
+	pthread_join(thread, NULL);
+	exit(0);
+}
+
diff --git a/tests/mmap/test b/tests/mmap/test
index 6b1de55..89badda 100755
--- a/tests/mmap/test
+++ b/tests/mmap/test
@@ -1,7 +1,7 @@
 #!/usr/bin/perl
 
 use Test;
-BEGIN { plan tests => 30}
+BEGIN { plan tests => 32}
 
 $basedir = $0;  $basedir =~ s|(.*)/[^/]*|$1|;
 
@@ -68,6 +68,12 @@ ok($result, 0);
 $result = system "runcon -t test_execmem_t $basedir/mprotect_stack 2>&1";
 ok($result);
 
+# Test success and failure for thread execstack, independent of execmem.
+$result = system "runcon -t test_execstack_t $basedir/mprotect_stack_thread";
+ok($result, 0);
+$result = system "runcon -t test_execmem_t $basedir/mprotect_stack_thread 2>&1";
+ok($result);
+
 # Test success and failure for file execute on mmap w/ file shared mapping.
 $result = system "runcon -t test_file_rwx_t $basedir/mmap_file_shared $basedir/temp_file";
 ok($result, 0);
-- 
2.8.0

Re: [RFC][PATCH] selinux: apply execstack check on thread stacks

[Nick Kralevich]

Thanks!

On Wed, Apr 6, 2016 at 12:57 PM, Stephen Smalley
<stephen.smalley@gmail.com> wrote:
> The execstack check was only being applied on the main
> process stack.  Thread stacks allocated via mmap were
> only subject to the execmem permission check.  Augment
> the check to apply to the current thread stack as well.
> Note that this does NOT prevent making a different thread's
> stack executable.
>
> Suggested-by: Nick Kralevich <nnk@google.com>

Acked-By: Nick Kralevich <nnk@google.com>

> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  security/selinux/hooks.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a9ca5ee..0271be4 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3465,8 +3465,9 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
>                     vma->vm_end <= vma->vm_mm->brk) {
>                         rc = cred_has_perm(cred, cred, PROCESS__EXECHEAP);
>                 } else if (!vma->vm_file &&
> -                          vma->vm_start <= vma->vm_mm->start_stack &&
> -                          vma->vm_end >= vma->vm_mm->start_stack) {
> +                          ((vma->vm_start <= vma->vm_mm->start_stack &&
> +                            vma->vm_end >= vma->vm_mm->start_stack) ||
> +                           vma_is_stack_for_task(vma, current))) {
>                         rc = current_has_perm(current, PROCESS__EXECSTACK);
>                 } else if (vma->vm_file && vma->anon_vma) {
>                         /*
> --
> 2.8.0
>



-- 
Nick Kralevich | Android Security | nnk@google.com | 650.214.4037