On 9/25/26 11:53, Lorenzo Stoakes (ARM) wrote: > On Fri, Sep 25, 2026 at 12:28:49AM -0700, Suren Baghdasaryan wrote: >> On Thu, Sep 17, 2026 at 9:25 AM Lorenzo Stoakes (ARM) <[email protected]> >> wrote: >>> >>> When a user requests an mmap_action be performed in mmap_prepare, this >>> involves populating the VMA range with data. >>> >>> However, if the VMA is mergeable, it might then mistakenly be merged with >>> another VMA without having populated the range. >>> >>> Every mmap action currently available sets VMA flags such that the VMA >>> cannot be merged. >>> >>> However, to ensure that no future mmap action falls foul of this, assert >>> that this is the case upon mmap_prepare validation. >>> >>> Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]> >> >> Reviewed-by: Suren Baghdasaryan <[email protected]> > > Thanks! > >> >>> --- >>> mm/vma.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/mm/vma.c b/mm/vma.c >>> index d6ed10cefc8f..62f2ce1ad5a1 100644 >>> --- a/mm/vma.c >>> +++ b/mm/vma.c >>> @@ -2809,6 +2809,15 @@ static int mmap_validate(unsigned long prev_start, >>> unsigned long prev_end, >>> int mmap_prepare_validate(const struct vm_area_desc *prev_desc, >>> const struct vm_area_desc *desc) >>> { >>> + /* >>> + * It is not valid to execute mmap actions for VMAs which can be >>> merged, >>> + * as any such merge would leave portions of the mapping incorrectly >>> + * unmapped. >>> + */ >>> + if (vma_flags_can_merge(&desc->vma_flags) && >>> + WARN_ON_ONCE(desc->action.type != MMAP_NOTHING)) >> >> Any reason you chose this "if (A && WARN_ON_ONCE(B))" pattern instead >> of a simpler "if (WARN_ON_ONCE(A && B))"? Unless there are races >> between A and B updates, I think these would be equivalent, right? > > They are equivalent yes! > > So it's more of a style thing, as that reads less clearly to me: > > if (WARN_ON_ONCE(vma_flags_can_merge(&desc->vma_flags) && > desc->action.type != MMAP_NOTHING)) > > And this version highlights what is the broken thing here (action type set) a > little more clearly though obviously that's only if mergeable so arguable. > > I don't have a really strong opinion though I can change it if you prefer?
Looks clearer to me like that Acked-by: David Hildenbrand (Arm) <[email protected]> -- Cheers, David
