* Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> [220713 13:50]:
> * Hugh Dickins <hughd@xxxxxxxxxx> [220713 11:56]:
> > On Wed, 13 Jul 2022, Liam Howlett wrote:
> > > * David Hildenbrand <david@xxxxxxxxxx> [220713 04:34]:
> > > > On 12.07.22 16:24, Liam Howlett wrote:
> > > > > When building with C=1, the maple tree had some rcu type mismatch &
> > > > > locking mismatches in the destroy functions. There were cosmetic only
> > > > > since this happens after the nodes are removed from the tree.
> > > > >
> > > > > Fixes: f8acc5e9581e (Maple Tree: add new data structure)
> > > > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > > > Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> > > >
> > > > Sorry to say, but the fixes become hard to follow (what/where/why). :)
> > > >
> > > > I guess it's time for a new series soon. Eventually it makes sense to
> > > > send the fixes as reply to the individual problematic patches. (instead
> > > > of fixes to commit ids that are not upstream)
> > > >
> > > > [yes, I'll do more review soon :) ]
> > >
> > > I appreciate the feedback, it's much better than yelling into the void.
> > > I have one more fix in the works - for __vma_adjust() of all functions
> > > so that'll be impossible to follow anyways :) I'll work on a v11 to
> > > include that last one.
> >
> > Please do also post the incremental for that "one more fix" once it's
> > ready: I have been keeping up with what you've been posting so far,
> > folding them into my debugging here, and believe we have made some but
> > still not enough progress on the bugs I hit. Folding in one more fix
> > will be easy for me, advancing to v11 of a 69-part patchset will be...
> > dispiriting.
>
>
> Okay, thanks. I don't think it will fix your outstanding issues but it
> is necessary to fix case 6 of vma_merge() on memory allocation failure
> as reported by syzbot.
Hugh,
Please find attached the last outstanding fix for this series. It
covers a rare failure scenario that one of the build bots reported.
Ironically, it changes __vma_adjust() more than the rest of the series,
but leaves the locking in the existing order.
Thanks,
Liam
From 081995cf1347406cb8be8c7ce11fbb256158c83e Mon Sep 17 00:00:00 2001
From: "Liam R. Howlett" <Liam.Howlett@xxxxxxxxxx>
Date: Wed, 13 Jul 2022 10:13:34 -0400
Subject: [PATCH 1/1] mm/mmap: Fix __vma_adjust() issue on memory failure
In case 6 of vma_merge, two VMAs will be freed, but when allocation
fails after the first __vma_adjust() completes and jumps back to the
"again" label, then the second VMA is not merged. Upon returning from
the __vma_adjust() call with an error code, the calling process assumes
the first VMA is still valid, but it has been freed.
Reported-by: syzbot+68771c0e74f7bb7804e5@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: d3ccd17e7c96 ("mm: start tracking VMAs with maple tree")
Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
---
mm/mmap.c | 42 ++++++++++--------------------------------
1 file changed, 10 insertions(+), 32 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index f25f53d7600d..5ed06870a3f3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -713,8 +713,6 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
next_next = find_vma(mm, next->vm_end);
VM_WARN_ON(remove_next == 2 &&
end != next_next->vm_end);
- /* trim end to next, for case 6 first pass */
- end = next->vm_end;
}
exporter = next;
@@ -762,7 +760,7 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
return error;
}
}
-again:
+
vma_adjust_trans_huge(orig_vma, start, end, adjust_next);
if (mas_preallocate(&mas, vma, GFP_KERNEL)) {
@@ -853,6 +851,9 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
if (remove_next && file) {
__remove_shared_vm_struct(next, file, mapping);
+ if (remove_next == 2)
+ __remove_shared_vm_struct(next_next, file, mapping);
+
} else if (insert) {
/*
* split_vma has split insert from vma, and needs
@@ -880,47 +881,24 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
}
if (remove_next) {
+again:
if (file) {
uprobe_munmap(next, next->vm_start, next->vm_end);
fput(file);
}
if (next->anon_vma)
anon_vma_merge(vma, next);
+
mm->map_count--;
mpol_put(vma_policy(next));
- BUG_ON(vma->vm_end < next->vm_end);
+ if (remove_next != 2)
+ BUG_ON(vma->vm_end < next->vm_end);
vm_area_free(next);
- /*
- * In mprotect's case 6 (see comments on vma_merge),
- * we must remove another next too. It would clutter
- * up the code too much to do both in one go.
- */
- if (remove_next != 3) {
- /*
- * If "next" was removed and vma->vm_end was
- * expanded (up) over it, in turn
- * "next->prev->vm_end" changed and the
- * "vma->next" gap must be updated.
- */
- next = next_next;
- } else {
- /*
- * For the scope of the comment "next" and
- * "vma" considered pre-swap(): if "vma" was
- * removed, next->vm_start was expanded (down)
- * over it and the "next" gap must be updated.
- * Because of the swap() the post-swap() "vma"
- * actually points to pre-swap() "next"
- * (post-swap() "next" as opposed is now a
- * dangling pointer).
- */
- next = vma;
- }
if (remove_next == 2) {
- mas_reset(&mas);
+ /* Case 6 (see vma_merge comments). Clean up next_next. */
remove_next = 1;
- end = next->vm_end;
+ next = next_next;
goto again;
}
}
--
2.35.1
