Re: [PATCH 1/2] mm/vma: fix gap check for unmapped_area with VM_GROWSDOWN

[Wei Yang <richard.weiyang@gmail.com>]

On Mon, Jan 27, 2025 at 12:08:04PM +0000, Lorenzo Stoakes wrote:
>You have a subject line of 'fix gap check for unmapped_area with
>VM_GROWSDOWN'. I'm not sure this is quite accurate.
>
>I don't really have time to do a deep dive (again, this is why it's so
>important to give a decent commit message - explaining under what _real
>world_ circumstances this will be used etc.).
>
>But anyway, it seems it will only be the case if MMF_TOPDOWN is not set in
>the mm flags, which usually requires an mmap compatibility mode to achieve
>unless the arch otherwise forces it.
>
>And these arches would be ones where the stack grows UP, right? Or at least
>ones where this is possible?
>
>So already we're into specifics - either arches that grow the stack up, or
>ones that intentionally use the old mmap compatibility mode are affected.
>
>This happens in:
>
>[ pretty much all unmapped area callers ]
>-> vm_unmapped_area()
>-> unmapped_area() (if !(info->flags & VM_UNMAPPED_AREA_TOPDOWN)
>
>Where VM_UNMAPPED_AREA_TOPDOWN is only not set in the circumstances
>mentioned above.
>
>So, for this issue you claim is the case to happen, you have to:
>
>1. Either be using a stack grows up arch, or enabling an mmap()
>compatibility mode.
>2. Also set MAP_GROWSDOWN on the mmap() call, which is translated to
>VM_GROWSDOWN.
>
>We are already far from 'fix gap check for VM_GROWSDOWN' right? I mean I
>don't have the time to absolutely dive into the guts of this, but I assume
>this is correct right?
>
>I'm not saying we shouldn't address this, but it's VITAL to clarify what
>exactly it is you're tackling.
>

Thanks for taking a look.

If my understanding is correct, your concern here is the case here never
happen in real world.

  We are searching a gap bottom-up, while the vma wants top-down.

This maybe possible to me. Here is my understanding, (but maybe not correct).

We have two separate flags affecting the search:

  * mm->flags:      MMF_TOPDOWN  or not
  * vma->vm_flags:  VM_GROWSDOWN or VM_GROWSUP

To me, they are independent.

For mm->flags, arch_pick_mmap_layout() could set/clear MMF_TOPDOWN it based on
the result of mmap_is_legacy(). Even we provide a sysctl file
/proc/sys/vm/legacy_vm_layout for configuration.


For vma->vm_flags, for general, VM_STACK is set to VM_GROWSDOWN by default.
And we use the flag in __bprm_mm_init() and setup_arg_pages().

So to me the case is real and not a very specific one.

But maybe I missed some background. Would you mind telling me the miss part,
if it is not too time wasting?

-- 
Wei Yang
Help you, Help me