Re: firefox + flash exit -> ddb

[Chuck Silvers <chuq%chuq.com@localhost>]

On Mon, Jan 02, 2012 at 01:14:50AM +0100, Thomas Klausner wrote:
> On Thu, Dec 08, 2011 at 07:51:34AM -0800, Chuck Silvers wrote:
> > I saw this problem pretty regularly earlier this year, but I haven't seen it
> > in the last two months or so.  if someone could send me a crash dump from
> > one of these crashes, that would be great.  I never managed to get a dump
> > back when it was happening to me.
> 
> I've just reproduced this bug with my 5.99.58 kernel.
> It doesn't happen everytime, but when firefox runs for a longer time,
> the chances are pretty good. Visiting a flash site is not a guarantee for
> the bug.
> 
> Sadly, when I "reboot 0x104"ed, I got a recursive panic.
> 
> Here's the backtrace of the original panic:
> panic: kernel diagnostic assertion "anon != NULL && anon->an_ref !=
> failed: file "/archive/cvs/src/sys/uvm/uvm_amap.c", line 718
> fatal breakpoint trap in supervisor mode
> trap type 1 code 0 rip ... cs 8 rflags 246 cr2 ... cpl
> rsp ...
> Stopped in pid 765.5103 (xulrunner-bin) at netbsd:breakpoint+0x5: leave
> breakpint
> vpanic
> kern_assert
> awap_wipeout
> uvm_unmap_detach
> uvmspace_free
> exit1
> sigexit
> postsig
> lwp_userret
> syscall
> 
[...]
> Does this tell you anything?

alas, nothing more than I knew already.  I did manage to get a dump of
this problem on one of my systems some time back, but it wasn't enlightening.
more recently, PR 46096 is the same problem again, and the dump I got from that
gave a few more clues, since that was with a DEBUG kernel.

what happens is that at some point we add more entries to the amap than
it has room for, so we overran the am_slots[] array.  normally this goes
undetected until the amap is freed (most often by amap_wipeout() when the
process exits), but the KMEM_SIZE debugging code notice it earlier when
the amap was being reallocated in amap_extend() since the KMEM_SIZE debug info
was clobbered by the array overrun.  the only way this could happen is if 
various fields of the amap got out of sync.  we already have some
assertions about the consistency of these fields, but those apparently
aren't catching the problem.  the attached patch adds some more checks
to hopefully catch the corruption earlier.  could you try running with that
with at least DIAGNOSTIC (and preferably DEBUG) enabled?

if you could also set ddb.onpanic=0, that should make getting a dump more 
likely.

-Chuck

Index: sys/uvm/uvm_amap.c
===================================================================
RCS file: /home/chs/netbsd/cvs/src/sys/uvm/uvm_amap.c,v
retrieving revision 1.105
diff -u -p -r1.105 uvm_amap.c
--- sys/uvm/uvm_amap.c  27 Jan 2012 19:48:41 -0000      1.105
+++ sys/uvm/uvm_amap.c  17 Mar 2012 16:06:08 -0000
@@ -300,6 +300,56 @@ amap_free(struct vm_amap *amap)
        UVMHIST_LOG(maphist,"<- done, freed amap = 0x%x", amap, 0, 0, 0);
 }
 
+#ifdef DEBUG
+
+static void
+amap_print(struct vm_amap *amap)
+{
+       printf("amap %p\n", amap);
+       printf("lock %p ref %d flags 0x%x\n", amap->am_lock, amap->am_ref, 
amap->am_flags);
+       printf("maxslot %d nslot %d nused %d\n", amap->am_maxslot, 
amap->am_nslot, amap->am_nused);
+       printf("slots %p\n", amap->am_slots);
+       printf("bckptr %p\n", amap->am_bckptr);
+       printf("anon %p\n", amap->am_anon);
+       printf("ppref %p\n", amap->am_ppref);
+}
+
+static void
+amap_check(int which, struct vm_amap *amap, vsize_t addsize, int flags, struct 
vm_amap *oamap)
+{
+       int i, slot, count;
+
+       KASSERT(amap->am_nused <= amap->am_nslot);
+       KASSERT(amap->am_nslot <= amap->am_maxslot);
+
+       for (i = 0; i < amap->am_nused; i++) {
+               slot = amap->am_slots[i];
+               KASSERT(amap->am_bckptr[slot] == i);
+               KASSERT(amap->am_anon[slot] != NULL);
+       }
+       count = 0;
+       for (i = 0; i < amap->am_nslot; i++) {
+               if (amap->am_anon[i]) {
+                       count++;
+                       KASSERT(amap->am_slots[amap->am_bckptr[i]] == i);
+               }
+       }
+       if (count != amap->am_nused) {
+               printf("which %d\n", which);
+               printf("count %d nused %d\n", count, amap->am_nused);
+               printf("amap %p addsize 0x%lx flags 0x%x\n", amap, addsize, 
flags);
+               printf("old values\n");
+               amap_print(oamap);
+               printf("new values\n");
+               amap_print(amap);
+       }
+       KASSERT(count == amap->am_nused);
+}
+
+#else
+#define amap_check(which, amap, addsize, flags, oamap)
+#endif
+
 /*
  * amap_extend: extend the size of an amap (if needed)
  *
@@ -325,6 +375,8 @@ amap_extend(struct vm_map_entry *entry, 
        const km_flag_t kmflags =
            (flags & AMAP_EXTEND_NOWAIT) ? KM_NOSLEEP : KM_SLEEP;
 
+       struct vm_amap oamap;
+
        UVMHIST_FUNC("amap_extend"); UVMHIST_CALLED(maphist);
 
        UVMHIST_LOG(maphist, "  (entry=0x%x, addsize=0x%x, flags=0x%x)",
@@ -337,6 +389,7 @@ amap_extend(struct vm_map_entry *entry, 
         */
 
        amap_lock(amap);
+       amap_check(1, amap, addsize, flags, NULL);
        KASSERT(amap_refs(amap) == 1); /* amap can't be shared */
        AMAP_B2SLOT(slotmapped, entry->end - entry->start); /* slots mapped */
        AMAP_B2SLOT(slotadd, addsize);                  /* slots to add */
@@ -489,6 +542,7 @@ amap_extend(struct vm_map_entry *entry, 
         * memory fragmentation by allocating them in separate blocks.
         */
 
+       oamap = *amap;
        amap_unlock(amap);
 
        if (slotneed >= UVM_AMAP_LARGE) {
@@ -524,6 +578,9 @@ amap_extend(struct vm_map_entry *entry, 
                return ENOMEM;
        }
        amap_lock(amap);
+
+       amap_check(2, amap, addsize, flags, &oamap);
+
        KASSERT(amap->am_maxslot < slotneed);
 
        /*
@@ -611,6 +668,8 @@ amap_extend(struct vm_map_entry *entry, 
        oldnslots = amap->am_maxslot;
        amap->am_maxslot = slotalloc;
 
+       amap_check(3, amap, addsize, flags, &oamap);
+
        uvm_anon_freelst(amap, tofree);
 
        kmem_free(oldsl, oldnslots * sizeof(*oldsl));
@@ -888,6 +947,7 @@ amap_copy(struct vm_map *map, struct vm_
                        continue;
                KASSERT(amap->am_anon[lcv]->an_lock == srcamap->am_lock);
                KASSERT(amap->am_anon[lcv]->an_ref > 0);
+               KASSERT(amap->am_nused < amap->am_maxslot);
                amap->am_anon[lcv]->an_ref++;
                amap->am_bckptr[lcv] = amap->am_nused;
                amap->am_slots[amap->am_nused] = lcv;
@@ -1489,6 +1549,7 @@ amap_add(struct vm_aref *aref, vaddr_t o
                }
        } else {
                KASSERT(amap->am_anon[slot] == NULL);
+               KASSERT(amap->am_nused < amap->am_maxslot);
                amap->am_bckptr[slot] = amap->am_nused;
                amap->am_slots[amap->am_nused] = slot;
                amap->am_nused++;