Closed Bug 515799 Opened 15 years ago Closed 15 years ago

reading freed memory in nsViewManager::ViewToWidget

Categories

(Core :: Web Painting, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 528025

People

(Reporter: jseward, Assigned: vlad)

Details

(Whiteboard: [sg:critical?] use-after-free)

The following read-after-free error was found when running mochitests-plain through Valgrind's Memcheck tool. It appeared in between these two messages: 703 INFO Running /tests/browser/base/content/test/test_bug364677.html... LoadPlugin: failed to initialize shared library /usr/local/lib/flashplayer10/libflashpla yer.so [/usr/local/lib/flashplayer10/libflashplayer.so: wrong ELF class: ELFCLASS32] and 704 INFO TEST-PASS | /tests/browser/base/content/test/test_bug364677.html | Feed served as text/xml without a channel/link should have been sniffed This is with mozilla-central of about 24 hours ago, r32351, release build, flags -g -O, on 64-bit Linux. The actual complaint occurred only once, and is this: Invalid read of size 4 at 0x598FBC6: nsViewManager::ViewToWidget(nsView*, nsView*, nsRect const&) const (nsIDeviceContext.h:284) by 0x59909B3: nsViewManager::UpdateWidgetArea(nsView*, nsIWidget*, nsRegion const&, nsView*) (nsViewManager.cpp:784) by 0x5990B6A: nsViewManager::UpdateView(nsIView*, nsRect const&, unsigned i nt) (nsViewManager.cpp:815) by 0x598D8F0: nsViewManager::UpdateView(nsIView*, unsigned int) (nsViewMana ger.cpp:586) by 0x598F085: nsViewManager::RemoveChild(nsIView*) (nsViewManager.cpp:1393) by 0x598A748: nsView::~nsView() (nsView.cpp:224) by 0x598979E: nsIView::Destroy() (nsView.cpp:294) by 0x598F3E2: nsViewManager::~nsViewManager() (nsViewManager.cpp:180) by 0x598D7DC: nsViewManager::Release() (nsViewManager.cpp:226) by 0x5EB3F33: nsCOMPtr_base::~nsCOMPtr_base() (nsCOMPtr.cpp:81) by 0x569D076: DocumentViewerImpl::~DocumentViewerImpl() (nsCOMPtr.h:469) by 0x56964A8: DocumentViewerImpl::Release() (nsDocumentViewer.cpp:567) Address 0x2063bb48 is 8 bytes inside a block of size 128 free'd at 0x4C23A9E: operator delete(void*) (vg_replace_malloc.c:346) by 0x5E1B457: nsThebesDeviceContext::~nsThebesDeviceContext() (nsThebesDevi ceContext.cpp:350) by 0x5E19EBE: nsThebesDeviceContext::Release() (nsThebesDeviceContext.cpp:2 94) by 0x56AE019: nsPresContext::~nsPresContext() (nsPresContext.cpp:306) by 0x56ABA8D: nsPresContext::Release() (nsPresContext.cpp:319) by 0x5EB4010: nsCOMPtr_base::assign_with_AddRef(nsISupports*) (nsCOMPtr.h:4 56) by 0x569CF07: DocumentViewerImpl::Destroy() (nsCOMPtr.h:640) by 0x569C066: DocumentViewerImpl::Show() (nsDocumentViewer.cpp:1879) by 0x56AA98F: nsPresContext::EnsureVisible() (nsPresContext.cpp:1584) by 0x56B15ED: PresShell::UnsuppressAndInvalidate() (nsPresShell.cpp:4502) by 0x56B1707: PresShell::UnsuppressPainting() (nsPresShell.cpp:4544) by 0x56AF12D: PresShell::sPaintSuppressionCallback(nsITimer*, void*) (nsPre sShell.cpp:2673)
I forgot to add this: out of three apparently identical mochitest runs, I only picked this up twice. So I'm wondering if it might involve some timing dependency (a race). Not saying it does, just that it might do.
jst: this might be related to the flash thing you're chasing?
I don't think the flash message is related here; I think that's just a "couldn't load flash .so" message. In my quick look at this, ViewToWidget is calling AppUnitsPerDevPixel() which returns mAppUnitsPerDevPixel from the device context. mContext is set to null in nsViewManager::~nsViewManager, but at the end of the function; Destory on the root view is called first. .. though now that I look, I see that nsViewManager doesn't hold a strong ref to the device context, so when DocumentViewerImpl::Destroy() is called, it destroys mDeviceContext (sets to nsnull), but mViewManager is still alive.. and it still has that same pointer. So yeah, there's a bug there.
Whiteboard: [sg:critical?] use-after-free
roc, is my analysis above correct? And if so, what's a good fix? Just change the bare pointer to a nsCOMPtr?
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
I don't know whether we need to advisory this or not -- the original bare nsIDeviceContext* pointer comes all the way from CVS, but I don't know if it it just became a problem recently or whether it's been here all along.
Group: core-security
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.