Support redirection of kernel32.dll for hooking function

VERIFIED FIXED in Firefox 36

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla38
x86
Windows 8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36+ verified, firefox37+ verified, firefox38 verified)

Details

()

Attachments

(1 attachment)

follow up bug 1112709.

kernel32.dll and advapi32.dll from Windows 7 redirect to kernelbase.dll etc using  jmp [disp32].

We should support this redirection of NoSpacePatcher and DetourPatcher.
We should resolve redirected address if function entry is jmp [disp32 (import table)].  It can hook by NoSpacePatcher.
Attachment #8549381 - Flags: review?(dmajor)
As a continuation of bug 1112709, this means the Flash protected-mode hook doesn't work yet on Windows 8.1.
Comment on attachment 8549381 [details] [diff] [review]
Support rediretion

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

I agree that this sounds better than hooking both the -A and -W functions.

::: xpcom/build/nsWindowsDllInterceptor.h
@@ +205,5 @@
>      return true;
>    }
> +
> +private:
> +  byteptr_t ResolveRedirectedAddress(const byteptr_t aOriginalFunction)

Could be static
Attachment #8549381 - Flags: review?(dmajor) → review+
https://hg.mozilla.org/mozilla-central/rev/d8dcfe4ca9f1
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
This issue is verified fixed on Nightly 38.0a1 (2015-01-19) using:
 * Windows 8.1 32-bit,
 * Windows 8.1 64-bit (Surface Pro 2).

I had mixed results while testing Windows 8 32-bit though. I used two machines to test it - on one of them the fix didn't work at all, on the other it worked 100% of the times. Detailed information is available in this etherpad [1], line 33.

Makoto, what's your take on this?

[1] https://etherpad.mozilla.org/FlashProtectedMode
Flags: needinfo?(m_kato)
(In reply to Andrei Vaida, QA [:avaida] from comment #6)
> This issue is verified fixed on Nightly 38.0a1 (2015-01-19) using:
>  * Windows 8.1 32-bit,
>  * Windows 8.1 64-bit (Surface Pro 2).
> 
> I had mixed results while testing Windows 8 32-bit though. I used two
> machines to test it - on one of them the fix didn't work at all, on the
> other it worked 100% of the times. Detailed information is available in this
> etherpad [1], line 33.
> 
> Makoto, what's your take on this?
> 
> [1] https://etherpad.mozilla.org/FlashProtectedMode

I cannot reproduce this on my Windows 8 VM.  Is reproduced environment clean environment (no application or anti-virus install etc)?

Or if you can reproduce this on virtual machine, could you share it?
Flags: needinfo?(m_kato) → needinfo?(andrei.vaida)
(In reply to Makoto Kato (:m_kato) from comment #7)
> I cannot reproduce this on my Windows 8 VM.  Is reproduced environment clean
> environment (no application or anti-virus install etc)?
> 
> Or if you can reproduce this on virtual machine, could you share it?

The test environments I used were not clean, other applications were installed on them but they were *not in use* at the time. I tested this fix again on two additional machines as well, on both of them Nightly 38.0a1 (2015-01-20) was *still affected*.

Here are the complete test results for Windows 8, across all the test machines I used:
- Windows 8 32-bit (AMD Radeon HD 7700 Series): FAIL
- Windows 8 32-bit (NVIDIA GeForce 620): PASS
- Windows 8 32-bit (AMD Radeon HD 6450): FAIL
- Windows 8 32-bit (ATI Radeon 3000): FAIL

I don't have a vm installed at this point, but I can setup one if you think it's necessary - nevertheless, I think that the most relevant results are those from environments that are as close to a random end user's machine as possible.
Flags: needinfo?(andrei.vaida)
Could you force a crash with the CrashMe extension [1] on the failing machine and share the crash ID. It will tell us the exact version of your system DLLs and may provide other clues also.

[1] http://ted.mielczarek.org/mozilla/crashme.html
Flags: needinfo?(andrei.vaida)
(In reply to David Major [:dmajor] (UTC+13) from comment #9)
> Could you force a crash with the CrashMe extension [1] on the failing
> machine and share the crash ID. It will tell us the exact version of your
> system DLLs and may provide other clues also.
> 
> [1] http://ted.mielczarek.org/mozilla/crashme.html

Sure, here are the crash IDs:
 * bp-b566a752-f5e0-4ba9-9f91-cecdb2150122
 * bp-392382ec-cb1c-436e-be11-6da942150122
 * bp-2aeb1e66-695b-4e7a-9348-91df32150122

Let me know if you need anything else.
Flags: needinfo?(andrei.vaida)
I don't see anything obvious that would lead to a failure of the hook. The opcode patterns are fine. It's going to be difficult to get much further without a live debugger.
Makoto, could you fill the uplift requests to aurora & beta? Thanks
Flags: needinfo?(m_kato)
Comment on attachment 8549381 [details] [diff] [review]
Support rediretion

Approval Request Comment
[Feature/regressing bug #]:N/A
[User impact if declined]:
dom.ipc.plugins.flash.disable-protected-mode doesn't work on Windows 8.1.
This preference is turned on from Firefox 37.

[Describe test coverage new/current, TreeHerder]:
landed in m-c and verified by QA team (comment #6)

[Risks and why]: 
too low.  This adds anylze jmp [disp32] only.

[String/UUID change made/needed]:
None
Flags: needinfo?(m_kato)
Attachment #8549381 - Flags: approval-mozilla-aurora?
Comment on attachment 8549381 [details] [diff] [review]
Support rediretion

Approval Request Comment
[Feature/regressing bug #]:
Related bug is bug 1108035, 1119941, and 1120993.

[User impact if declined]:
dom.ipc.plugins.flash.disable-protected-mode doesn't work on Windows 8.1.
This preference is turn off on Firefox 36, but this discussion is bug 1120993

[Describe test coverage new/current, TreeHerder]:
Landed in m-c and verified by QA team (comment #6)

[Risks and why]: 
too low.  This adds anylze jmp [disp32] only.

[String/UUID change made/needed]:
None
Attachment #8549381 - Flags: approval-mozilla-beta?
Attachment #8549381 - Flags: approval-mozilla-beta?
Attachment #8549381 - Flags: approval-mozilla-beta+
Attachment #8549381 - Flags: approval-mozilla-aurora?
Attachment #8549381 - Flags: approval-mozilla-aurora+
Since uplift requests were approved for this patch, I assume that the Windows 8 related issue (see Comment 6) will be treated in a separate bug. Should I file one or do we have it already?
(In reply to Andrei Vaida, QA [:avaida] from comment #15)
> Since uplift requests were approved for this patch, I assume that the
> Windows 8 related issue (see Comment 6) will be treated in a separate bug.
> Should I file one or do we have it already?

Could you file new bug?
Blocks: 1126185
(In reply to Makoto Kato (:m_kato) from comment #18)
> Could you file new bug?

Sure, I've filed Bug 1126185 on this matter.

I'm gonna go ahead and mark this fix as verified on Windows 8.1, per Comment 6. I'll follow up with test results for m-a and m-b as soon as possible.
Verified fixed on Firefox 36.0b4 (build1: 20150126151838) and Aurora 37.0a2 (2015-01-26), using:
 - Windows 8.1 32 bit,
 - Windows 8.1 64 bit (Surface Pro 2),
with Shockwave Flash 16.0 r0 (16.0.0.296).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.