* Bert Karwatzki <spasswolf@xxxxxx> [230223 15:36]: > Am Donnerstag, dem 23.02.2023 um 12:04 -0800 schrieb Andrew Morton: > > (switched to email. Please respond via emailed reply-to-all, not via > > the > > bugzilla web interface). > > > > On Wed, 22 Feb 2023 18:27:37 +0000 bugzilla-daemon@xxxxxxxxxx wrote: > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=217061 > > > > > > --- Comment #5 from spasswolf@xxxxxx --- > > > This fixes the problem for me in next-20230220 > > > > > > diff --git a/mm/mprotect.c b/mm/mprotect.c > > > index 1d4843c97c2a..f70f9a7b545e 100644 > > > --- a/mm/mprotect.c > > > +++ b/mm/mprotect.c > > > @@ -832,6 +832,7 @@ static int do_mprotect_pkey(unsigned long > > > start, size_t > > > len, > > > if (error) > > > break; > > > > > > + tmp = prev->vm_end; > > > nstart = tmp; > > > prot = reqprot; > > > } > > > > > > Next I'll try to construct a simpler testcase without steam and > > > stellaris. > > > > Thanks for figuring this out. > > > > Liam, this has been bisected to 2286a6914c776e ("mm: change > > mprotect_fixup to vma iterator"). Please take a look? > > > While a still don't have a simple example for this bug I can at least > explain what it takes to trigger it: > The memory area for which mprotect is called must have (at least) 3 > vm_area_struct of which the first to must be mergeable. Then the > following happens in the for_each_vma_range loop inside > do_mprotect_pkey: > At the beggining of the loop vma points to the first vm_area_struct. > Then mprotect_fixup is called and merges our first two vm_area_structs > but tmp still points to the beginning of the second vm_area_struct. At > the beginning of the next iteration vma already points to the third > vm_area_struct and so the check vma->vm_start != tmp gives a false > error. > Setting tmp=prev->vm_end after mprotect fixup sets tmp to the > beginning of the next unprocessed vma and fixes this error. > Thank you for the analysis. Just for clarity, tmp is used to ensure there isn't a hole in the range we are iterating over. I don't particularly like how this was done, but I tried (unsuccessfully) not to break it. It looks like I've missed the case where a merge succeeds and so tmp is now out of sync with the iterator. I would change this tmp = prev->vm_end idea slightly and use the vma iterator end location: tmp = vma_iter_end(&vmi); I'll send out a patch once I test this. Thanks, Liam
