Java related crash with deleted pointer in esx

RESOLVED FIXED in Firefox 12

Status

()

Core
Plug-ins
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: bc, Assigned: jst)

Tracking

(Blocks: 2 bugs, {crash})

9 Branch
mozilla11
x86
Windows 7
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox8- wontfix, firefox9- wontfix, firefox10+ wontfix, firefox11- wontfix, firefox12+ fixed, firefox13+ fixed, firefox-esr1012+ fixed, blocking1.9.2 needed, status1.9.2 ?)

Details

(Whiteboard: [sg:critical][qa-], crash signature, URL)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 577852 [details]
crash report

0. enable Java(TM) Platform SE 6 U29
1. enable popups
2. disable malware detector
3. http://www.sheru100.in/view/index585.html
4. shutdown

http://cpm1.jmxy.com:899/cpm/tg_href.aspx?tg_d=www.ads80.com will be loaded which is reported as a malware attack site.

Nightly/11 Windows 7
bp-f1059123-a28f-499c-9003-5ed772111129
Crash Report [@ mozalloc_abort(char const* const) | NS_DebugBreak_P | mozilla::plugins::PluginModuleChild::ShouldContinueFromReplyTimeout() ] 

See also bug 681385

Automation hit this with ecx pointing to deleted memory. I wasn't able to reproduce locally though.

Operating system: Windows NT
                  6.1.7601 Service Pack 1
CPU: x86
     GenuineIntel family 6 model 44 stepping 2
     2 CPUs

Crash reason:  EXCEPTION_ACCESS_VIOLATION_WRITE
Crash address: 0xffffffffddddde1d

Thread 0 (crashed)
 0  xul.dll!nsNPAPIPluginInstance::InvalidateOwner() [nsNPAPIPluginInstance.cpp : 1431 + 0x3]
    eip = 0x6f9ed2ea   esp = 0x0031b128   ebp = 0x0031b12c   ebx = 0x00000001
    esi = 0x04ae00e8   edi = 0xffffff87   eax = 0xdddddddd   ecx = 0xdddddddd
    edx = 0x00000001   efl = 0x00210246
    Found by: given as instruction pointer in context
 1  xul.dll!nsDummyJavaPluginOwner::Destroy() [nsGlobalWindow.cpp : 483 + 0x11]
    eip = 0x6f28450a   esp = 0x0031b134   ebp = 0x0031b138
    Found by: call frame info
 2  xul.dll!nsGlobalWindow::FreeInnerObjects(int) [nsGlobalWindow.cpp : 1311 + 0x14]
    eip = 0x6f2874ff   esp = 0x0031b140   ebp = 0x0031b168
    Found by: call frame info
 3  xul.dll!nsGlobalWindow::SetDocShell(nsIDocShell *) [nsGlobalWindow.cpp : 2397 + 0x10]
    eip = 0x6f28bde4   esp = 0x0031b170   ebp = 0x0031b1c8
    Found by: call frame info
 4  xul.dll!nsDocShell::Destroy() [nsDocShell.cpp : 4611 + 0x1c]
    eip = 0x6f82852a   esp = 0x0031b1d0   ebp = 0x0031b22c
    Found by: call frame info
 5  xul.dll!nsFrameLoader::Finalize() [nsFrameLoader.cpp : 573 + 0x18]
    eip = 0x6efbc3a1   esp = 0x0031b234   ebp = 0x0031b248
    Found by: call frame info
(Reporter)

Comment 1

6 years ago
Created attachment 577853 [details]
log
Can we capture the testcase? If the site is already flagged as a malware site the contents might go away at any time.
Keywords: testcase-wanted
Whiteboard: [sg:critical]
(Reporter)

Comment 3

6 years ago
I'll try again. It was late and the attempts I made using wget didn't seem to catch anything useful.
(Assignee)

Updated

6 years ago
Assignee: nobody → jst
blocking1.9.2: --- → ?
status-firefox10: --- → affected
status-firefox11: --- → affected
status-firefox8: --- → wontfix
status-firefox9: --- → wontfix
tracking-firefox10: --- → +
tracking-firefox11: --- → +
tracking-firefox8: --- → -
tracking-firefox9: --- → -
(Assignee)

Comment 4

6 years ago
Created attachment 578507 [details] [diff] [review]
Possible fix.

It's unclear exactly how this crash happens, but I did find this inconsistency where we can create a dummy java plugin (which we create when accessing window.java n friends) and then navigate switch documents in the window w/o creating a new inner. In that case we end up with a dummy java plugin whose owner has a reference to the old document rather than the new one. This fixes that case, but I have not had a chance to test whether this fixes the problem here.

Bob, any chance you can test this out? I can spin a try build if that helps.
(Reporter)

Comment 5

6 years ago
dveditz was right that this might be fleeting. I couldn't get anything to reproduce except for some really high cpu usage. I can retry again. A try build would be cool, though I can build my own with the patch if needed. I'll get to this after I take care of some other things this morning.
Have we seen evidence of this on the 1.9.2 branch, or is it a relatively-new-build crash?
blocking1.9.2: ? → needed
status1.9.2: --- → ?
(Reporter)

Comment 7

6 years ago
I have seen only once instance of a related crash with signature mozilla::plugins::PluginModuleChild::ShouldContinueFromReplyTimeout. Socorro shows 300 in the last week though: https://crash-stats.mozilla.com/query/query?do_query=1&query_type=contains&query=mozilla%3A%3Aplugins%3A%3APluginModuleChild%3A%3AShouldContinueFromReplyTimeout

I have seen only one instance of a related crash with signature nsNPAPIPluginInstance::InvalidateOwner. There are no instances of this in Socorro.

I haven't been able to reproduce locally.

The original SocorroRecord where the crash automation obtained the url to test had signature mozilla::widget::WindowHook::Lookup(unsigned int) which has ~3000 crashes in the last week. https://crash-stats.mozilla.com/query/query?do_query=1&query_type=contains&query=mozilla%3A%3Awidget%3A%3AWindowHook%3A%3ALookup 

See 556524. There were two mozilla::widget::WindowHook::Lookup at sheru100 in Nov/Dec. I'll try to collect a set of urls to retest to see if I can get more information.

Johnny, without more information want to go ahead and see if this helps in other ways? Perhaps the fix will affect other "unrelated" signatures.
(Assignee)

Comment 8

6 years ago
Comment on attachment 578507 [details] [diff] [review]
Possible fix.

Yeah, I think we should just land this patch, requesting review.
Attachment #578507 - Flags: review?(joshmoz)
Looks like this applies to 1.9.2 as well
blocking1.9.2: needed → .26+
status1.9.2: ? → wanted
status-firefox12: --- → affected
tracking-firefox12: --- → +

Comment 10

6 years ago
Comment on attachment 578507 [details] [diff] [review]
Possible fix.

Review of attachment 578507 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsGlobalWindow.cpp
@@ +2177,5 @@
> +          // this window.
> +          mDummyJavaPluginOwner->Destroy();
> +          mDummyJavaPluginOwner = nsnull;
> +
> +          mDidInitJavaProperties = PR_FALSE;

How come we don't set mDidInitJavaProperties to false in the other places where we destroy the dummy instance, like in "nsGlobalWindow::FreeInnerObjects"?
Attachment #578507 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 11

6 years ago
All other places that tear down the dummy java plugin we're at the very end of the lifetime of the window, so it doesn't matter. But for consistency's sake it wouldn't hurt to do the same there as well.
blocking1.9.2: .26+ → needed
status1.9.2: wanted → ?
status-firefox12: affected → ---
tracking-firefox12: + → ---
Target Milestone: --- → mozilla11
(Assignee)

Comment 12

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/025fa101a51b
(Assignee)

Comment 13

6 years ago
https://hg.mozilla.org/mozilla-central/rev/025fa101a51b

Leaving this bug open though, as it's not clear whether this patch actually changes things here, so we should track this in crash-stats etc for a while before we can tell.
Probably not much chance of getting this into Fx10 at this point, but it is a safe contained fix. Are there enough crashes that we could decide in a couple of days?
status-firefox10: affected → wontfix
status-firefox12: --- → affected
tracking-firefox12: --- → +
If this patch did help then we should get it into the ESR at some point.
status-firefox-esr10: --- → affected
tracking-firefox-esr10: --- → ?

Updated

6 years ago
tracking-firefox-esr10: ? → .1+

Updated

5 years ago
tracking-firefox-esr10: .1+ → 11+
Setting the ESR tracking flag to 12+ since this hasn't yet been uplifted to 11.
tracking-firefox-esr10: 11+ → 12+
(Assignee)

Comment 17

5 years ago
Bob, any signs of this left? Or can we even tell?
status-firefox11: affected → wontfix
status-firefox13: --- → affected
tracking-firefox11: + → -
tracking-firefox13: --- → +
(Reporter)

Comment 18

5 years ago
I haven't seen any signs of this but it is hard to tell. I wonder if this is related at all to bug 724781
(Assignee)

Comment 19

5 years ago
Ok, calling this fixed then. We can always reopen this bug if additional information emerges.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox12: affected → fixed
status-firefox13: affected → fixed
Resolution: --- → FIXED
[Triage Comment]
Can this patch be landed as is to ESR branch?  Please nominate for approval-mozilla-esr10 if so.  See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for details.
qa- until testcase-wanted is satisfied
Whiteboard: [sg:critical] → [sg:critical][qa-]
Comment on attachment 578507 [details] [diff] [review]
Possible fix.

This applies to ESR10
Attachment #578507 - Flags: approval-mozilla-esr10?

Updated

5 years ago
Attachment #578507 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
(Assignee)

Comment 23

5 years ago
https://hg.mozilla.org/releases/mozilla-esr10/rev/c8a79e73f18f
status-firefox-esr10: affected → fixed
Nothing much to verify here since we've never been able to reproduce the problem again.
Group: core-security
Keywords: testcase-wanted
You need to log in before you can comment on or make changes to this bug.