Open Bug 933826 Opened 6 years ago Updated 6 years ago

IPC: crash with PLayerTransaction::Msg_Update [@nsRegion::SetToElements]

Categories

(Core :: IPC, defect, critical)

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

()

People

(Reporter: posidron, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Attachments

(1 file)

Attached file fuzzing-session
Tested with an opt/non-debug build of https://github.com/posidron/mozilla-central/commit/26121cb
Okay so,

#0  nsRegion::SetToElements (this=0xaf227208, aCount=<optimized out>) at ../../../../mozilla-central/gfx/src/nsRegion.cpp:305
#1  0xb4fd56da in nsRegion::SetEmpty (this=0xaf227208) at ../../../../mozilla-central/gfx/src/nsRegion.h:148

We can see that nsRegion::SetEmpty is defined as:

  146   void SetEmpty ()
  147   {
  148     SetToElements (0);
  149     mBoundRect.SetRect (0, 0, 0, 0);
  150   }

Considering that the code of SetToElements() that is called is:

   273 // Adjust the number of rectangles in region.
   274 // Content of rectangles should be changed by caller.
   275 
   276 void nsRegion::SetToElements (uint32_t aCount)
   277 {
   278   if (mRectCount < aCount)        // Add missing rectangles
   279   {
   280     uint32_t InsertCount = aCount - mRectCount;
   281     mRectCount = aCount;
   282     RgnRect* pPrev = &mRectListHead;
   283     RgnRect* pNext = mRectListHead.next;
   284 
   285     while (InsertCount--)
   286     {
   287       mCurRect = new RgnRect;
   288       mCurRect->prev = pPrev;
   289       pPrev->next = mCurRect;
   290       pPrev = mCurRect;
   291     }
   292 
   293     pPrev->next = pNext;
   294     pNext->prev = pPrev;
   295   } else
   296   if (mRectCount > aCount)        // Remove unnecessary rectangles
   297   {
   298     uint32_t RemoveCount = mRectCount - aCount;
   299     mRectCount = aCount;
   300     mCurRect = mRectListHead.next;
   301 
   302     while (RemoveCount--)
   303     {
   304       RgnRect* tmp = mCurRect;
   305       mCurRect = mCurRect->next;
   306       delete tmp;
   307     }
   308 
   309     mRectListHead.next = mCurRect;
   310     mCurRect->prev = &mRectListHead;
   311   }
   312 }

So from the header file we have aCount = 0 ; and the crash happens at line 305, meaning mRectCount was at least >= 1.

It would mean that the list mRectListHead is not in sync somehow: either its content gets outdated, or it had garbage inside.
However, the stack mentions:

#2  0xb4fd57ac in nsRegion::Copy (this=0xaf227208, aRegion=...) at ../../../../mozilla-central/gfx/src/nsRegion.cpp:585

Which, is:

   579 nsRegion& nsRegion::Copy (const nsRegion& aRegion)
   580 {
   581   if (&aRegion == this)
   582     return *this;
   583 
   584   if (aRegion.mRectCount == 0)
   585     SetEmpty ();
   586   else
   587   {
   588     SetToElements (aRegion.mRectCount);
   589 
   590     const RgnRect* pSrc = aRegion.mRectListHead.next;
   591     RgnRect* pDest = mRectListHead.next;
   592 
   593     while (pSrc != &aRegion.mRectListHead)
   594     {
   595       *pDest = *pSrc;
   596 
   597       pSrc  = pSrc->next;
   598       pDest = pDest->next;
   599     }
   600 
   601     mCurRect = mRectListHead.next;
   602     mBoundRect = aRegion.mBoundRect;
   603   }
   604 
   605   return *this;
   606 }

So I'm not sure to correctly understand what happens here ; the region that we are copying is with mRectCount = 0, and we call SetEmpty(0), but mRectCount of the nsRegion object is different.
(In reply to Alexandre LISSY :gerard-majax from comment #2)
> So I'm not sure to correctly understand what happens here ; the region that
> we are copying is with mRectCount = 0, and we call SetEmpty(0), but
> mRectCount of the nsRegion object is different.

aRegion.mRectCount is zero when we're inside Copy but that doesn't mean that this->mRectCount was zero then.
(In reply to Benoit Girard (:BenWa) from comment #3)
> (In reply to Alexandre LISSY :gerard-majax from comment #2)
> > So I'm not sure to correctly understand what happens here ; the region that
> > we are copying is with mRectCount = 0, and we call SetEmpty(0), but
> > mRectCount of the nsRegion object is different.
> 
> aRegion.mRectCount is zero when we're inside Copy but that doesn't mean that
> this->mRectCount was zero then.

So it's legit ? And then it would mean the issue lies in the list.
(gdb) bt full
#0  nsRegion::SetToElements (this=0xaf227208, aCount=<optimized out>) at ../../../../mozilla-central/gfx/src/nsRegion.cpp:305
        tmp = 0x0
        RemoveCount = <optimized out>

So the list is out of sync ...
You need to log in before you can comment on or make changes to this bug.