Skip to content

Commit 1f4b030

Browse files
lorenzo-stoakesgregkh
authored andcommitted
mm/vma: reset VMA iterator on commit_merge() OOM failure
commit 0cf4b16 upstream. While an OOM failure in commit_merge() isn't really feasible due to the allocation which might fail (a maple tree pre-allocation) being 'too small to fail', we do need to handle this case correctly regardless. In vma_merge_existing_range(), we can theoretically encounter failures which result in an OOM error in two ways - firstly dup_anon_vma() might fail with an OOM error, and secondly commit_merge() failing, ultimately, to pre-allocate a maple tree node. The abort logic for dup_anon_vma() resets the VMA iterator to the initial range, ensuring that any logic looping on this iterator will correctly proceed to the next VMA. However the commit_merge() abort logic does not do the same thing. This resulted in a syzbot report occurring because mlockall() iterates through VMAs, is tolerant of errors, but ended up with an incorrect previous VMA being specified due to incorrect iterator state. While making this change, it became apparent we are duplicating logic - the logic introduced in commit 41e6ddc ("mm/vma: add give_up_on_oom option on modify/merge, use in uffd release") duplicates the vmg->give_up_on_oom check in both abort branches. Additionally, we observe that we can perform the anon_dup check safely on dup_anon_vma() failure, as this will not be modified should this call fail. Finally, we need to reset the iterator in both cases, so now we can simply use the exact same code to abort for both. We remove the VM_WARN_ON(err != -ENOMEM) as it would be silly for this to be otherwise and it allows us to implement the abort check more neatly. Link: https://lkml.kernel.org/r/20250606125032.164249-1-lorenzo.stoakes@oracle.com Fixes: 47b16d0 ("mm: abort vma_modify() on merge out of memory failure") Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Reported-by: syzbot+d16409ea9ecc16ed261a@syzkaller.appspotmail.com Closes: https://lore.kernel.org/linux-mm/6842cc67.a00a0220.29ac89.003b.GAE@google.com/ Reviewed-by: Pedro Falcato <pfalcato@suse.de> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> Cc: Jann Horn <jannh@google.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 560c3b5 commit 1f4b030

File tree

1 file changed

+8
-19
lines changed

1 file changed

+8
-19
lines changed

mm/vma.c

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -836,32 +836,18 @@ static struct vm_area_struct *vma_merge_existing_range(struct vma_merge_struct *
836836
err = dup_anon_vma(next, vma, &anon_dup);
837837
}
838838

839-
if (err)
840-
goto abort;
841-
842839
/*
843840
* In nearly all cases, we expand vmg->vma. There is one exception -
844841
* merge_right where we partially span the VMA. In this case we shrink
845842
* the end of vmg->vma and adjust the start of vmg->next accordingly.
846843
*/
847844
expanded = !merge_right || merge_will_delete_vma;
848845

849-
if (commit_merge(vmg, adjust,
850-
merge_will_delete_vma ? vma : NULL,
851-
merge_will_delete_next ? next : NULL,
852-
adj_start, expanded)) {
853-
if (anon_dup)
854-
unlink_anon_vmas(anon_dup);
855-
856-
/*
857-
* We've cleaned up any cloned anon_vma's, no VMAs have been
858-
* modified, no harm no foul if the user requests that we not
859-
* report this and just give up, leaving the VMAs unmerged.
860-
*/
861-
if (!vmg->give_up_on_oom)
862-
vmg->state = VMA_MERGE_ERROR_NOMEM;
863-
return NULL;
864-
}
846+
if (err || commit_merge(vmg, adjust,
847+
merge_will_delete_vma ? vma : NULL,
848+
merge_will_delete_next ? next : NULL,
849+
adj_start, expanded))
850+
goto abort;
865851

866852
res = merge_left ? prev : next;
867853
khugepaged_enter_vma(res, vmg->flags);
@@ -873,6 +859,9 @@ static struct vm_area_struct *vma_merge_existing_range(struct vma_merge_struct *
873859
vma_iter_set(vmg->vmi, start);
874860
vma_iter_load(vmg->vmi);
875861

862+
if (anon_dup)
863+
unlink_anon_vmas(anon_dup);
864+
876865
/*
877866
* This means we have failed to clone anon_vma's correctly, but no
878867
* actual changes to VMAs have occurred, so no harm no foul - if the

0 commit comments

Comments
 (0)