Hi all, I forward a Bugzilla report [1]. As you may know, many developers don't take a look on Bugzilla (especially linux-kernel@xxxxxxxxxxxxxxxxxxxxxx as no one subscribes to the generic component). The original reporter (Ilija Tovilo) writes: > 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; > } Then he gives bug reproducers: > 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. > > 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); > } Thanks. [1]: https://bugzilla.kernel.org/show_bug.cgi?id=218258 -- An old man doll... just what I always wanted! - Clara
Attachment:
signature.asc
Description: PGP signature
