Last Comment Bug 681924 - HookSetWindowLongPtr is called several times
: HookSetWindowLongPtr is called several times
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Windows Vista
: -- normal (vote)
: ---
Assigned To: Makoto Kato [:m_kato]
:
:
Mentors:
Depends on:
Blocks: 653361 684215
  Show dependency treegraph
 
Reported: 2011-08-25 05:16 PDT by Makoto Kato [:m_kato]
Modified: 2011-09-07 07:51 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix v1 (2.22 KB, patch)
2011-08-25 05:17 PDT, Makoto Kato [:m_kato]
no flags Details | Diff | Splinter Review
fix v2 (6.17 KB, patch)
2011-08-26 04:22 PDT, Makoto Kato [:m_kato]
jmathies: review+
Details | Diff | Splinter Review

Description Makoto Kato [:m_kato] 2011-08-25 05:16:35 PDT
When I had added multiple injenction support by bug 653361, plugin-container crashed.  Because HookSetWindowLongPtr is called several times.

HookSetWindowLongPtr should not inject after 2nd times.  (because return address is static!)
Comment 1 Makoto Kato [:m_kato] 2011-08-25 05:17:28 PDT
Created attachment 555707 [details] [diff] [review]
fix v1
Comment 2 Makoto Kato [:m_kato] 2011-08-26 04:22:42 PDT
Created attachment 555996 [details] [diff] [review]
fix v2
Comment 3 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-08-26 10:14:00 PDT
Comment on attachment 555996 [details] [diff] [review]
fix v2

I think you want jimmm to look over this.
Comment 4 Jim Mathies [:jimm] 2011-08-29 07:00:16 PDT
Originally this was a passive error, no harm came from calling AddHook more than once. If we've changed this, I wonder if there is some way we can detect it in AddHook and assert?
Comment 5 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-09-02 07:28:25 PDT
I went ahead and landed this as a bustage fix for Bug 684215.

http://hg.mozilla.org/mozilla-central/rev/0664108eb19d
Comment 6 Makoto Kato [:m_kato] 2011-09-02 07:43:49 PDT
(In reply to Jim Mathies [:jimm] from comment #4)
> Originally this was a passive error, no harm came from calling AddHook more
> than once. If we've changed this, I wonder if there is some way we can
> detect it in AddHook and assert?

No assertion on current code.  But now, glandium has added TestDllIntercept test that can inject APIs.

Note You need to log in before you can comment on or make changes to this bug.