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
