Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(319)

Issue 21044: Hands off the intercept if 'unpatch' fails... (Closed)

Created:
11 years, 10 months ago by amit
Modified:
9 years ago
Reviewers:
Dean McNamee, ananta
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Hands off the intercept if 'unpatch' fails If IATPatchFunction::Unpatch fails during RestoreImportedFunction it means that we cannot safely unpatch the import address table patch. In this case its better to be hands off the intercept as trying to unpatch again in the destructor of IATPatchFunction is not going to be any safer. In real world, when we patch a plugin's SetCursor, we intercept npswf.dll's IAT entry of SetCursor. It seems that our unpatch fails when the plugin ref count goes to 0. It could be because some one else has patched on top of us. Then, during CRT uninitialization at process shutdown, the destructor of IATPatchFunction is called. It detects that we haven't unpatched yet and tries to unpatch. But at this time the plugin DLL is unloaded and the IAT thunk is invalid. There's no point in trying to unpatch unloaded DLL's IAT :) BUG=6886 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=9142

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 1

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -5 lines) Patch
M base/iat_patch.cc View 1 2 1 chunk +10 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
amit
11 years, 10 months ago (2009-02-04 16:14:36 UTC) #1
Dean McNamee
Ok, that makes some sense. It seems like we should figure out why the unpatch ...
11 years, 10 months ago (2009-02-04 16:42:07 UTC) #2
Dean McNamee
The fact that that function returns an error code makes me want to DCHECK it, ...
11 years, 10 months ago (2009-02-04 16:52:35 UTC) #3
Dean McNamee
LG, thakns
11 years, 10 months ago (2009-02-04 17:09:11 UTC) #4
ananta
11 years, 10 months ago (2009-02-04 17:13:04 UTC) #5
LGTM

http://codereview.chromium.org/21044/diff/5/1004
File base/iat_patch.cc (right):

http://codereview.chromium.org/21044/diff/5/1004#newcode211
Line 211: DCHECK(NO_ERROR == error);
Please add a comment here as to why this might fail.

Powered by Google App Engine
This is Rietveld 408576698