On Tue, 20 Jun 2017, Linus Torvalds wrote: > On Tue, Jun 20, 2017 at 8:26 AM, Dave Jones <[email protected]> wrote: > > > Hugh Dickins (1): > > > mm: larger stack guard gap, between vmas > > > > This seems to be buggered. > > > > 002331 00000396712307 0 2 kernel BUG at mm/mmap.c:1963! > > 002332 00000396712414 0 4 invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC > > 002333 00000396712541 0 4 CPU: 0 PID: 4572 Comm: trinity-c41 Not tainted > > 4.12.0-rc6-think+ #1 > > 002336 00000396712959 0 4 RIP: 0010:unmapped_area_topdown+0xa5/0x170 > > Dave, do you have instructions for Hugh to recreate that with trinity > (or perhaps some way to generate a test-case from trinity?). Or does > it trigger easily by just running trinity? > > I'm in China right now, and will be traveling again this afternoon, so > I probably can't look at it myself until later, but hopefully Hugh has > the cycles to follow up in it.. > > Hugh? The changes to unmapped_area_topdown() look trivial, but > obviously there's something wrong there. The code decodes to > > 49 39 c0 cmp %rax,%r8 > 76 d0 jbe 0xfffffffffffffffb > * 0f 0b ud2 <-- trapping instruction > > so from the > > VM_BUG_ON(gap_end < gap_start); > > we have gap_start/end in %r8 and %rax respectively, which are: > > R08: 00007f7d54673000 > RAX: 00007f7d543d6000 > > so yes, gap_start is bigger than gap_end there by quite a degree (more > than the 1MB of the gap size unless I looked at it wrong). > > Hmm. Maybe it's this: > > /* Check if current node has a suitable gap */ > gap_end = vm_start_gap(vma); > if (gap_end < low_limit) > return -ENOMEM; > if (gap_start <= high_limit && gap_end - gap_start >= length) > goto found; > > where it used to be that gap_end was guaranteed to be after gap_start, > but that's no longer true. We have > > gap_start = vma->vm_prev ? vm_end_gap(vma->vm_prev) : 0; > gap_end = vm_start_gap(vma); > > and by using MAP_FIXED, you can end up in the situation that > "vma->vm_prev" is closer to vma than the gap size. > > So now gap_end - gap_start will underflow, and then the logic that > does "goto found" thinks it found a hole that is larger than > "length", when in actual fact it found a "negative-size" hole. > > So maybe that "goto found" condition should have an additional test > for "gap_end > gap_start"? > > Or maybe I'm just hallucinating and missed something. Hugh, Oleg, > Michal, can you take another look and double-check this logic?
My first impression is that you've got right to the heart of it, before I even started looking. I'll go over that area more carefully now, in case there are other such instances, and post a test patch for Dave perhaps to try - but probably he's shut down now, so I'll then grab a trinity, and see what luck I have with it. Hugh
