On Thu, Jul 20, 2023 at 4:28 AM Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote: > On 2023/7/19 23:25, Paul Moore wrote: > > On Wed, Jul 19, 2023 at 6:23 AM Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote: > >> On 2023/7/19 17:02, Christian Göttsche wrote: > >>> On Wed, 19 Jul 2023 at 09:40, Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote: > >>>> > >>>> Use the helpers to simplify code. > >>>> > >>>> Cc: Paul Moore <paul@xxxxxxxxxxxxxx> > >>>> Cc: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> > >>>> Cc: Eric Paris <eparis@xxxxxxxxxxxxxx> > >>>> Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx> > >>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> > >>>> --- > >>>> security/selinux/hooks.c | 7 ++----- > >>>> 1 file changed, 2 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > >>>> index d06e350fedee..ee8575540a8e 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)) { > >>> > >>> This seems to change the condition from > >>> > >>> vma->vm_start >= vma->vm_mm->start_brk && vma->vm_end <= vma->vm_mm->brk > >>> > >>> to > >>> > >>> vma->vm_start <= vma->vm_mm->brk && vma->vm_end >= vma->vm_mm->start_brk > >>> > >>> (or AND arguments swapped) > >>> > >>> vma->vm_end >= vma->vm_mm->start_brk && vma->vm_start <= vma->vm_mm->brk > >>> > >>> Is this intended? > >> > >> The new condition is to check whether there is intersection between > >> [startbrk,brk] and [vm_start,vm_end], it contains orignal check, so > >> I think it is ok, but for selinux check, I am not sure if there is > >> some other problem. > > > > This particular SELinux vma check is see if the vma falls within the > > heap; can you confirm that this change preserves this? > > Yes, within is one case of new vma scope check. Thanks for the confirmation. -- paul-moore.com