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:
- 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?