Bug 218258 - SELinux mprotect EACCES/execheap for memory segment directly adjacent to heap
Summary: SELinux mprotect EACCES/execheap for memory segment directly adjacent to heap
Status: NEW
Alias: None
Product: Linux
Classification: Unclassified
Component: Kernel (show other bugs)
Hardware: All Linux
: P3 normal
Assignee: Virtual assignee for kernel bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-12 13:18 UTC by Ilija Tovilo
Modified: 2023-12-14 11:53 UTC (History)
1 user (show)

See Also:
Kernel Version:
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Ilija Tovilo 2023-12-12 13:18:26 UTC
Hi! We're running into an issue with SELinux where mprotect will result in a EACCES due to the execheap policy since Kernel 6.6. This happens when the mmap-ed segment lies directly adjacent to the heap. I think this is caused by the following patch:

https://github.com/torvalds/linux/commit/68df1baf158fddc07b6f0333e4c81fe1ccecd6ff

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d06e350fedee5f..ee8575540a8efc 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3762,13 +3762,10 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
 	if (default_noexec &&
 	    (prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
 		int rc = 0;
-		if (vma->vm_start >= vma->vm_mm->start_brk &&
-		    vma->vm_end <= vma->vm_mm->brk) {
+		if (vma_is_initial_heap(vma)) {
 			rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
 					  PROCESS__EXECHEAP, NULL);
-		} else if (!vma->vm_file &&
-			   ((vma->vm_start <= vma->vm_mm->start_stack &&
-			     vma->vm_end >= vma->vm_mm->start_stack) ||
+		} else if (!vma->vm_file && (vma_is_initial_stack(vma) ||
 			    vma_is_stack_for_current(vma))) {
 			rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
 					  PROCESS__EXECSTACK, NULL);

Before this patch, selinux_file_mprotect would check whether the original start_brk and brk values lied within the vma segment. However, this does not match the vma_is_initial_heap implementation.

static inline bool vma_is_initial_heap(const struct vm_area_struct *vma)
{
       return vma->vm_start <= vma->vm_mm->brk &&
		vma->vm_end >= vma->vm_mm->start_brk;
}

This function checks whether vma overlaps with start_brk/brk. However, since the comparison includes equality this will also yield true if the segment lies directly before or after the heap. It's possible that equality is handling cases where start_brk == brk and start_brk == vm_start, but I'm not sure. In that case, the following patch might work.

static inline bool vma_is_initial_heap(const struct vm_area_struct *vma)
{
       return (vma->vm_start < vma->vm_mm->brk && vma->vm_end > vma->vm_mm->start_brk)
		|| vma->vm_start == vma->vm_mm->start_brk;
}

Thank you!
Comment 1 Ilija Tovilo 2023-12-12 14:10:00 UTC
Here's a small reproducer, which may be of help:

int main(void)
{
	const size_t size = 2 * 1024 * 1024;
	void *brk = sbrk(0);
	void *mem = mmap(brk, size, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS|MAP_FIXED, -1, 0);
	if (mem == MAP_FAILED) {
		fprintf(stderr, "Address %p wasn't free, try again.\n", brk);
		exit(1);
	}
	if (mprotect(mem, size, PROT_READ|PROT_EXEC) == -1) {
		fprintf(stderr, "mprotect() failed [%d] %s\n", errno, strerror(errno));
		exit(1);
	}
	munmap(mem, size);
}

Results in:

mprotect() failed [13] Permission denied

Adding some padding between the heap using `void *brk = sbrk(0) + size;` solves the problem.
Comment 2 Ilija Tovilo 2023-12-12 15:12:11 UTC
For completion, the inverse also fails, as expected.

#include <stdint.h>
#include <sys/mman.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>

static void *find_start_brk(void)
{
	uintptr_t start, end;
	FILE *f;
	char buffer[4096];
	f = fopen("/proc/self/maps", "r");
	while (fgets(buffer, 4096, f) && sscanf(buffer, "%lx-%lx", &start, &end) == 2) {
		if (strstr(buffer, "[heap]") != NULL) {
			return (void *)start;
		}
	}
	fclose(f);

	return 0;
}

int main(void)
{
	const size_t size = 2 * 1024 * 1024;
	void *start_brk = find_start_brk();
	void *mem = mmap(start_brk - size, size, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS|MAP_FIXED, -1, 0);
	if (mem == MAP_FAILED) {
		fprintf(stderr, "Address %p wasn't free, try again.\n", brk);
		exit(1);
	}
	if (mprotect(mem, size, PROT_READ|PROT_EXEC) == -1) {
		fprintf(stderr, "mprotect() failed [%d] %s\n", errno, strerror(errno));
		exit(1);
	}
	munmap(mem, size);
}
Comment 3 Bagas Sanjaya 2023-12-14 11:53:23 UTC
(In reply to Ilija Tovilo from comment #0)
> Hi! We're running into an issue with SELinux where mprotect will result in a
> EACCES due to the execheap policy since Kernel 6.6. This happens when the
> mmap-ed segment lies directly adjacent to the heap. I think this is caused
> by the following patch:
> 
> https://github.com/torvalds/linux/commit/
> 68df1baf158fddc07b6f0333e4c81fe1ccecd6ff
> 

Hi Ilija,

mm-stable tree should have the fix for this bug [1]. Please test.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-hotfixes-stable&id=d3bb89ea9c13e5a98d2b7a0ba8e50a77893132cb

Note You need to log in before you can comment on or make changes to this bug.