514583 - Crash upon returning from file picker if I right-clicked [@ _ftol2 - JS_NewNumberValue] due to Abylon Reader

Status: RESOLVED DUPLICATE of bug 533035

(Core :: JavaScript Engine, defect)

Description

[Jacob Wexler]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2

Repeatable 100%, regardless of visited sites. All add-ons are disabled. Crash report at
http://crash-stats.mozilla.com/report/index/c1c60b53-ee04-486d-8c0d-7ed362090903
Also tried V.3.6a2 - crashes just the same.

Reproducible: Always

Steps to Reproduce:
1. Open a few tabs (e.g. 3) with a displayed page in each tab.
2. Click on a file link in one of the displayed pages to download.
3. Inside the resulting "Save As" dialog box, right-click the mouse in order to create a new folder.
4. Cancel the operation by closing the dialog box.
5. Click on one of the tabs. FF crashes within a few seconds.

Comment 1

[timeless]

Signature	_ftol2
UUID	c1c60b53-ee04-486d-8c0d-7ed362090903
Time 	2009-09-03 15:31:40.720620
Uptime	223
Last Crash	4401 seconds before submission
Product	Firefox
Version	3.5.2
Build ID	20090729225027
Branch	1.9.1
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
CPU	x86
CPU Info	AuthenticAMD family 6 model 4 stepping 2
Crash Reason	EXCEPTION_FLT_INVALID_OPERATION
Crash Address	0x352865
User Comments	Crashes 100% of the cases whenever I have a few tabs open, try to download a file, and in the "Save As" dialog box I right-click mouse button for the pop-up menu (for creating a new directory etc.). Regradless if I go ahead and download, or cancel the "Save As" operation, when I next click on any of the tabs FF crashes. Win XP SP3, all add-ons are disabled.FF3.0 and 2.0 are OK.
Processor Notes 	
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	js3250.dll 	_ftol2 	
1 	js3250.dll 	JS_NewNumberValue 	js/src/jsapi.cpp:1897
2 	xul.dll 	XPCConvert::NativeData2JS 	js/src/xpconnect/src/xpcconvert.cpp:259
3 	xul.dll 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:2541
4 	xul.dll 	XPC_WN_GetterSetter 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1622
5 	js3250.dll 	js_Invoke 	js/src/jsinterp.cpp:1386
6 	js3250.dll 	js_InternalInvoke 	js/src/jsinterp.cpp:1447
7 	js3250.dll 	js_GetPropertyHelper 	js/src/jsobj.cpp:4333
8 	js3250.dll 	js_Interpret 	js/src/jsinterp.cpp:4451
9 	js3250.dll 	js_Invoke 	js/src/jsinterp.cpp:1394
10 	js3250.dll 	array_extra 	js/src/jsarray.cpp:3164
11 	js3250.dll 	array_forEach 	js/src/jsarray.cpp:3220
12 	js3250.dll 	js_Interpret 	js/src/jsinterp.cpp:5147
13 	js3250.dll 	js_Invoke 	js/src/jsinterp.cpp:1394
14 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1697
15 	xul.dll 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:569
16 	xul.dll 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
17 	xul.dll 	SharedStub 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
18 	xul.dll 	nsTimerImpl::Fire 	xpcom/threads/nsTimerImpl.cpp:428
19 	xul.dll 	xul.dll@0x89d01b 	

Filename 	Version 	Debug Identifier 	Debug Filename
Hook.dll 		3D9AD566f 	Hook.pdb
APMROle.DLL 	4.0.1.77 		
APMRLang.DLL 	4.0.1.40 		
APMRCMN32.DLL 	4.0.1.88 		
APMRTOOLS.DLL 	4.5.1.50 		

I didn't recognize APMR, it seems to be: http://www.abylonsoft.com/reader/

Could you try uninstalling it? 

What your crash means is that something runs as a shell extension and stomps on the process's cpu state, which is incredibly rude. If uninstalling reader "fixes" the problem, please let us know so we can contact the vendor and complain to them.

Comment 2

[Andreas Gal :gal]

Looks like a floating-point exception. I don't think this is exploitable, so leaving it open.

Comment 3

[Jacob Wexler]

@timeless - I am grateful for the pointer. I uninstalled the Abylon Reader and no more crashes. I installed the reader roughly a year ago to read PKCS certificates - FF 2.0 and 3.0 did not have the problem, but 3.5.2 did.

The Reader uninstallation appears to also resolve another problem that I and other forum's users had - the scrolling of large directories in the file "save as" dialog box was overshooting the intended target, jumping 2-3 scroll pages at a time. Now it does not seems to do that anymore. 

Thanks.

Comment 4

[Jacob Wexler]

Correction - the claim for the collateral fix by the Abylon removal of the overshoot page jumps when scrolling through the "save as" dialog box was premature. Still happening.

Comment 5

[Andreas Gal :gal]

timeless, I can't reproduce but I might be able to fix it. Can you help me testing patching with custom builds?

Comment 6

[timeless]

jacob: so is the crash going away?

gal: wrong target; historically these problems are caused by third party libraries changing the system precision. i'm not absolutely certain it's the right problem here, but i like being right :).

http://www.codercorner.com/FPUFun.htm

describes the general feature, i suppose.

the problem is that "fixing" "this" involves reseting the fpu word after any call to untrusted code, in our world, that's at least:
* plugins
* common dialogs (we are here)
* printing
* potentially gdi if we're incredibly unlucky

the best "fix" for this problem is to move js to a process which isn't attacked by foreign code :).

Comment 7

[Jacob Wexler]

Yes, the crash went away by uninstalling the Abylon Reader. 
My correction note referred only to my prior comment about the displayed page overshoot while scrolling the "Save As" dialog box.

Comment 8

[timeless]

thanks :)

Comment 9

[Jesse Ruderman on Windows]

_ftol2 is topcrash #105 or so.  The entry in http://people.mozilla.com/crash_analysis/20091026/20091026_Firefox_3.5.3-interesting-modules.txt for _ftol2 shows a bunch of correlations with things that sound like shell extensions.  So I bet just hitting the common-dialog case would help.

Should we file bugs for resetting the fpu control word in each case?  e.g. one bug for "Reset FPU control word after returning from Windows common dialogs".

Comment 10

[timeless]

we could try doing that for common cases. i'm not opposed to that. personally, i'd rather get those dialogs to run out of process (i made that request before a couple of times in talking to the ipc guys). there are a number of awful things that a shell extension can do (one is link against any version of CLR <http://blogs.msdn.com/oldnewthing/archive/2006/12/18/1317290.aspx>)

windows common dialogs are actually quite numerous, i'd rather one for the file picker dialogs, one for printing dialogs (this is kinda distinct from "printing" listed in comment 6). we can start with those two and see how things go.

"Dynamic Link Library Redirection" and:
http://www.codeguru.com/forum/archive/index.php/t-334750.html
sound like fun

--
note that comment 6's item "plugins" didn't clearly explain that random code can register for system events and then do work then, at which point we'd have to attempt to undo each time we process a native event (or something incredibly painful) - this is why we've generally skipped this pain.

--
one note, my link from comment 6 indicates that it's a per thread thing, which means where i wrote the best "fix", you could read "process" as "thread".

--
Yesterday I spoke with some of the Google people who work on one of their plugin things and they described a system where they have two processes which run in lockstep. I think we could take that approach to js if we wanted to (yes, it's stupid overhead) using threads (it'd probably break stack walking) - exit from js would be sent back to the frozen calling thread.

--
anyway. i think my mantra in the previous bug where this came up was "IPC will save me". I still believe.

Comment 11

[Jesse Ruderman]

IPC for file pickers sounds harder than resetting the fpu control word.  Are there actually plans to do IPC for them, or even a bug on that?

Comment 12

[timeless]

filepickers are actually incredibly simple to ipc, they have a very simple api and very limited amount of data to transfer. i specifically asked people working on ipc about it. i suspect we don't have a bug for it. when i last asked about it (with the intent of implementing it myself), the ipc people said things weren't really ready for people to do stuff with it.

resetting the fpu control word properly actually requires way more work since there are quite a few potential derailing points.

Comment 13

[Mike Shaver (:shaver emeritus)]

I don't understand: for a given simple API, isn't it at least as easy to insert FPU control word reset code after each call to it (maybe by a helper, if we call to the native file picker in more places than I hope we do) as it is to remote that simple API?

Comment 14

[timeless]

eh? what api?

do you want us to reset the fpu word each and every time we enter js from all threads? that's expensive.

there's no callback for "someone changed the fpu control word" (if there was, we could just panic when that happens and actually)...

we could do that. we could using the same sort of dll blocklisting trick try to catch anyone calling these functions (_control87, _controlfp)

http://msdn.microsoft.com/en-us/library/e9b52ceh(VS.71).aspx

it'd basically be:
as we load dlls, check to see if they have public exports for _control87/_controlfp. if they do, add a trampoline to those functions. the trampoline when called can check to see if the thread is the main thread and if so walk the stack to the caller, get its dll name, and write that name out to a file. the next run our blocklist would use those additional entries to veto the evil library.

back to the problem statement: any code can at any time that it's running on the main thread muck with the fpu control word. we could reset it each time through the event loop, but that'd be somewhat painful and won't help if the fpu is mucked by a gdi thing (or plugin, or insert your whatever hook here) and then we return and call js before we reach the event loop.

filepicker addons can be evil in all sorts of ways, they can crash, they can muck with the fpu control word, they can load in incompatible modules (there's a fun thing relating to shell extensions pulling in .NET [it's totally against the rules, but there was a sample component by someone working at microsoft which does it :o).

I wanted to remote the filepicker because of the crashy bit of shell extensions more than because of the fpu control word. It's just a happy coincidence that remoting it would fix that portion of this bug.

Probably most importantly... the file picker api is "simple", but it causes us to reenter gecko. or perhaps simply put, when I call the filepicker api, it pushes an event loop. So, sure, I can reset the fpu control word when it finishes, but that doesn't protect any gecko code that runs until it does.

http://msdn.microsoft.com/en-us/library/ms646927(VS.85).aspx