On 11/07/2023 18:50, Liam R. Howlett wrote: > apply_vma_lock_flags() calls mlock_fixup(), which could merge the VMA > after where the vma iterator is located. Although this is not an issue, > the next iteration of the loop will check the start of the vma to be > equal to the locally saved 'tmp' variable and cause an incorrect failure > scenario. Fix the error by setting tmp to the end of the vma iterator > value before restarting the loop. > > There is also a potential of the error code being overwritten when the > loop terminates early. Fix the return issue by directly returning when > an error is encountered since there is nothing to undo after the loop. > > Reported-by: Ryan Roberts <ryan.roberts@xxxxxxx> > Link: https://lore.kernel.org/linux-mm/50341ca1-d582-b33a-e3d0-acb08a65166f@xxxxxxx/ > Cc: <stable@xxxxxxxxxxxxxxx> > Fixes: 37598f5a9d8b ("mlock: convert mlock to vma iterator") > Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> Wow; thanks for the quick fix! I've tested this on arm64/v6.4 with the failing mm selftests that I reported: they are now passing. Tested-by: Ryan Roberts <ryan.roberts@xxxxxxx> > --- > mm/mlock.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/mm/mlock.c b/mm/mlock.c > index d7db94519884..0a0c996c5c21 100644 > --- a/mm/mlock.c > +++ b/mm/mlock.c > @@ -477,7 +477,6 @@ static int apply_vma_lock_flags(unsigned long start, size_t len, > { > unsigned long nstart, end, tmp; > struct vm_area_struct *vma, *prev; > - int error; > VMA_ITERATOR(vmi, current->mm, start); > > VM_BUG_ON(offset_in_page(start)); > @@ -498,6 +497,7 @@ static int apply_vma_lock_flags(unsigned long start, size_t len, > nstart = start; > tmp = vma->vm_start; > for_each_vma_range(vmi, vma, end) { > + int error; > vm_flags_t newflags; > > if (vma->vm_start != tmp) > @@ -511,14 +511,15 @@ static int apply_vma_lock_flags(unsigned long start, size_t len, > tmp = end; > error = mlock_fixup(&vmi, vma, &prev, nstart, tmp, newflags); > if (error) > - break; > + return error; > + tmp = vma_iter_end(&vmi); > nstart = tmp; > } > > - if (vma_iter_end(&vmi) < end) > + if (tmp < end) > return -ENOMEM; > > - return error; > + return 0; > } > > /*
