Crash in [@ ReadFile] with Beijing Kingsoft Security software
Categories
(External Software Affecting Firefox :: Other, defect, P1)
Tracking
(Fission Milestone:MVP, firefox-esr78 wontfix, firefox-esr91 wontfix, firefox89 wontfix, firefox90 wontfix, firefox91 wontfix, firefox93 wontfix, firefox94+ wontfix, firefox95+ fixed)
People
(Reporter: jcristau, Assigned: toshi)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
Crash report: https://crash-stats.mozilla.org/report/index/a99fa469-bb31-4bc6-bb81-5cdc60201129
Reason: EXCEPTION_ACCESS_VIOLATION_EXEC
Top 10 frames of crashing thread:
0 @0x246
1 kernelbase.dll ReadFile
2 kernelbase.dll WriteFile
3 kisfdpro64.dll kisfdpro64.dll@0x1b00ce
4 kernel32.dll ReadFileImplementation
5 kisfdpro64.dll kisfdpro64.dll@0x13eb3
6 kisfdpro64.dll kisfdpro64.dll@0x13b74
7 kisfdpro64.dll kisfdpro64.dll@0x13fab
8 kisfdpro64.dll kisfdpro64.dll@0x1b00ce
9 kisfdpro64.dll kisfdpro64.dll@0x1b00ce
We've seen this dll before, e.g. bug 1499194.
Reporter | ||
Comment 1•3 years ago
|
||
bp-49fb9a32-0b32-40b1-abda-647c40201129 is similar, different dll from the same vendor.
Comment 2•3 years ago
|
||
Why did this crash signature spike in late April and again in early June? The crashes are mostly in Beta and Nightly. The Beta crashes are not correlated with the Fission's Beta 90 experiment.
All Windows versions are affected, but 94% are Windows 7.
Reporter | ||
Comment 3•3 years ago
|
||
There's a strong correlation with the zh-cn locale, which accounts for over 80% of the recent crashes. (Perhaps not surprising given the dll's vendor.)
Comment 4•3 years ago
|
||
Spiking again ?
Updated•3 years ago
|
Comment 5•3 years ago
|
||
Unfortunately, this crash significantly affects Fission on Windows 7/8/8.1. We will probably need to delay Fission on Windows 7/8/8.1 until we can resolve this crash.
Comment 6•3 years ago
|
||
Tentatively assigning this bug to Toshi because he said he would to investigate.
The Kingsoft DLLs are kisfdpro64.dll
and kwsui64.dll
.
Comment 7•3 years ago
•
|
||
Here are versions of the crashing DLLs (from crash ping telemetry):
kisfdpro64.dll
2021.9.27.105
2021.9.14.34
2021.9.6.100
2021.8.26.99
2021.8.16.95
2021.8.4.91
2021.6.2.82
2021.4.26.78
2021.1.5.47
2020.12.9.38
kwsui64.dll
2021.9.14.102
2021.6.2.984
2020.4.10.24299
Assignee | ||
Comment 8•3 years ago
•
|
||
Luckily I was able to reproduce the same crash. Here are the steps.
- Download the installer of 金山毒覇 from https://www.duba.net/ and install it on Windows 7 (As of today, the version was 2021.9.27.105)
- Enable Fission on Nightly
- Set the homepage to https://www.newduba.cn/?f=foxdh&pru=1
- Launch Firefox
What happened is KERNELBASE!ReadFile
called NtReadFile
, but somehow it jumped to a near-null address. The interesting finding is that NtReadFile
was hooked to InterposedNtReadFile
by us. This may be a timing issue around the global variable gOriginalNtReadFile
, and enabling Fission changes the timing when NtReadFile
is called in the content process.
I'll debug this further, but I think blocking the module is not the right approach to this. IOInterposer's hooking of ntdll functions seems not stable. Because of bug 1666310, this hook is applied only in Nightly and early Beta for now. If this is a potential ship blocker of Fission, we should consider not enabling IOInterposer if Fission is enabled.
Updated•2 years ago
|
Assignee | ||
Comment 9•2 years ago
|
||
Here's what happens:
- Kingsoft (kwsprotect64.exe) hooks firefox.exe's entrypoint (firefox!wmainCRTStartup) and injects a code to load kwsui64.dll
- [Main thread] the entrypoint function loads kwsui64.dll via the injected code
- [Main thread] kwsui64.dll's DllMain creates a thread that starts from
kwsui64+0x3120
- [kwsui64's thread] kwsui64.dll calls
ReadFile
for their named pipe - [Main thread] our code applies IOInterposer's hooks (only in nightly and early beta)
There is a race between Step 4 and 5. When kwsui64's thread calls NtReadFile
, the register r11
is usually 0x246. On the other hand, our thread applies a hook whose pattern is mov r11, <target>; jmp r11
onto NtReadFile
. On Windows 7, this race magically gets the kwsui64's thread execute jmp r11
with the original value 0x246, resulting in this crash, execution AV at 0x246. This flow is the exact same on Windows 10, but somehow the crash never happens.
Any third-party module injected before InitPoisonIOInterposer
can trigger this problem with or without Fission. A possible solution is to apply the hooks earlier. I think we can do that with our blocklist's technique or chromium-sandbox technique.
Assignee | ||
Comment 10•2 years ago
|
||
I just found bug 1646804 proposing the same thing. We can land a mitigation for this bug and work on a long-term solution in bug 1646804.
Assignee | ||
Comment 11•2 years ago
|
||
Kingsoft's modules are injected before IOInterposer
and starts a thread that
calls NtReadFile
. It conflicts with the main thread where IOInterposer
hooks
NtReadFile
.
This patch is a mitigation until we make IOInterposer
compatible with such
third-party injections (bug 1646804).
Comment 12•2 years ago
|
||
Pushed by tkikuchi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2e03fdd340bd Not enable IOInterposer when Kingsoft Internet Security is installed. r=gerald
Comment 13•2 years ago
|
||
bugherder |
Comment 14•2 years ago
•
|
||
Your IOInterposer workaround checks for kwsui64.dll
, but some of the Kingsoft crashes include kisfdpro64.dll
but not kwsui64.dll
.
This crash report's modules include Kingsoft DLLs CORRECTION: I see now that my crash report search was incorrect.kwsui64.dll
and KSoftLaunch64.dll
, but not kisfdpro64.dll
.
https://crash-stats.mozilla.org/report/index/53b4451d-a18d-4677-b4c3-073e90211027#tab-modules
Should this patch also check for kisfdpro64.dll
?
Assignee | ||
Comment 15•2 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #14)
Should this patch also check for
kisfdpro64.dll
?
I think it's not needed for two reasons. The first reason is all the crashes coming from kisfdpro64.dll (like this) always have kwsui64.dll loaded. The second is our third-party modules ping shows kisfdpro64.dll is loaded through kwsui64.dll. So checking kwsui64.dll is enough to detect all Kingsoft users.
Comment 16•2 years ago
|
||
(In reply to Toshihito Kikuchi [:toshi] from comment #15)
I think it's not needed for two reasons. The first reason is all the crashes coming from kisfdpro64.dll (like this) always have kwsui64.dll loaded. The second is our third-party modules ping shows kisfdpro64.dll is loaded through kwsui64.dll. So checking kwsui64.dll is enough to detect all Kingsoft users.
Sounds good. I see now that my crash report search was incorrect.
I'm setting status-firefox94=wontfix. We don't need to uplift this IOInterposer fix to Firefox 94 Release because the IOInterposer is only enabled in Nightly and Early Beta.
Description
•