On Thu, Aug 8, 2024 at 7:09 AM Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote: > > > > On 2024/8/8 14:43, Kefeng Wang wrote: > > > > On 2024/8/8 9:10, Paul Moore wrote: > >> On Wed, Aug 7, 2024 at 5:26 PM Marc Reisner <reisner.marc@xxxxxxxxx> > >> wrote: > >>> > >>> It looks like this issue is still not fixed. There has been some > >>> investigation on going in this Bugzilla for Fedora: > >>> > >>> https://bugzilla.redhat.com/show_bug.cgi?id=2254434 > >> > >> FWIW, I recently learned there was also a report in the kernel > >> bugzilla about the same issue: > >> > >> https://bugzilla.kernel.org/show_bug.cgi?id=218258 > >> > >>> The behavior we are seeing is that when a process has no heap and > >>> mmap(2) is called with MAP_PRIVATE | MAP_ANONYMOUS, it allocates memory > >>> on the heap. > >>> > >>> If the address space returned by mmap(2) is later on made executable > >>> with mprotect(2), that triggers an execheap avc. > >> > >> ... > >> > >>> This has been reproduced on kernels >= 6.6. > > > > I try the reproducer, but fails to reproduce on my 6.6 > > After enable selinux then trigger the issue. > > >>> > >>> In reviewing the code, my best guess is that this is caused by the > >>> scenario where brk == start_brk not being handled, though I am not > > > > Is there vma_start,end and start_brk,brk infos, adding some debug print. > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 81fbfa5b80d4..b66c381c558e 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -3848,6 +3848,9 @@ static int selinux_file_mprotect(struct > > vm_area_struct *vma, > > if (vma_is_initial_heap(vma)) { > > rc = avc_has_perm(sid, sid, SECCLASS_PROCESS, > > PROCESS__EXECHEAP, NULL); > > + if (rc) > > + pr_err("DEBUG: vma_start,end=%lx,%lx, > > start_brk,brk=%lx,%lx\n", > > + vma->vm_start, vma->vm_end, > > vma->vm_mm->start_brk, vma->vm_mm->brk); > > } else if (!vma->vm_file && (vma_is_initial_stack(vma) || > > vma_is_stack_for_current(vma))) { > > rc = avc_has_perm(sid, sid, SECCLASS_PROCESS, > > > [ 3520.913952] DEBUG: pid=769674,comm=test-vma, > vma:start,end=1e9c000,21e9c000, start_brk,brk=2310000,2310000 > > > vma_start < start_brk = brk < vma_end > > > > >>> expert enough in kernel code to know. If the start address allocated > >>> by mmap is before the starting program break, and the end address is > >>> after the starting program break, then the avc will trigger. However, > >>> I don't know how mmap deals with determining an address and if it takes > >>> into account the program break, or if calling brk(2) later on will just > >>> pick a new location. > >> > >> I'm not a mm expert, but thankfully we have some on the To/CC line so > >> I'm hopeful they will be able to take a look and provide some insight. > > > > +Cc mmap maintainers too. > > > >> > >> To bring the relevant code into this email, prior to using the > >> vma_is_initial_heap() helper the SELinux execheap logic looked like > >> this: > >> > >> /* WORKING */ > >> if (vma->vm_start >= vma->vm_mm->start_brk && > >> vma->vm_end <= vma->vm_mm->brk) > >> /* execheap denial */ > >> > > vma range must be within heap. > > >> ... while the current vma_is_initial_heap() helper has logic that > >> looks like this: > >> > >> /* BUGGY */ > >> if (vma->vm_start < vma->vm_mm->brk && > >> vma->vm_end > vma->vm_mm->start_brk) > >> /* execheap denial */ > > vma range intersection with brk range will be rejected, and > there's a cross in above case, so execheap denial, I not sure it > should be allowed or not, hope maintainers give some comments here. It's a kernel regression so we need to revert the change to SELinux at least - can't start denying permission without a new/updated policy (e.g. making the change in the check conditional on a new policy capability).
