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

Issue 1475023004: Get module versions and types from in-memory images (Closed)

Created:
5 years ago by Mark Mentovai
Modified:
5 years ago
Reviewers:
scottmg
CC:
crashpad-dev_chromium.org
Base URL:
https://chromium.googlesource.com/crashpad/crashpad@master
Target Ref:
refs/heads/master
Project:
crashpad
Visibility:
Public.

Description

Get module versions and types from in-memory images Don't call GetFileVersionInfo(), which calls LoadLibrary() to be able to access the module's resources. Loading modules from the crashy process into the handler process can cause trouble. The Crashpad handler definitely doesn't want to run arbitrary modules' module initializer code. Since the VS_FIXEDFILEINFO needed is already in memory in the remote process' address space, just access it from there. BUG=crashpad:78 R=scottmg@chromium.org Committed: https://chromium.googlesource.com/crashpad/crashpad/+/5be8ce4ea0fe2345ce6c843c53432e07a22cca41

Patch Set 1 #

Total comments: 17

Patch Set 2 : Address review feedback #

Total comments: 1

Patch Set 3 : Address review feedback (2) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1003 lines, -155 lines) Patch
M snapshot/snapshot.gyp View 1 1 chunk +4 lines, -0 lines 0 comments Download
M snapshot/win/module_snapshot_win.cc View 2 chunks +1 line, -2 lines 0 comments Download
M snapshot/win/pe_image_reader.h View 1 6 chunks +46 lines, -29 lines 0 comments Download
M snapshot/win/pe_image_reader.cc View 1 10 chunks +166 lines, -93 lines 0 comments Download
M snapshot/win/pe_image_reader_test.cc View 4 chunks +60 lines, -5 lines 0 comments Download
A snapshot/win/pe_image_resource_reader.h View 1 1 chunk +179 lines, -0 lines 0 comments Download
A snapshot/win/pe_image_resource_reader.cc View 1 1 chunk +279 lines, -0 lines 0 comments Download
A snapshot/win/process_subrange_reader.h View 1 2 1 chunk +114 lines, -0 lines 0 comments Download
A snapshot/win/process_subrange_reader.cc View 1 1 chunk +104 lines, -0 lines 0 comments Download
M snapshot/win/system_snapshot_win.cc View 1 1 chunk +26 lines, -24 lines 0 comments Download
M util/numeric/checked_address_range.h View 2 chunks +8 lines, -0 lines 0 comments Download
M util/numeric/checked_address_range.cc View 2 chunks +8 lines, -0 lines 0 comments Download
M util/win/module_version.h View 1 1 chunk +8 lines, -2 lines 0 comments Download

Messages

Total messages: 10 (2 generated)
Mark Mentovai
If you know any more than this about the language search order, I’d be interested…
5 years ago (2015-11-25 22:10:46 UTC) #2
scottmg
https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_reader.cc File snapshot/win/pe_image_reader.cc (right): https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_reader.cc#newcode242 snapshot/win/pe_image_reader.cc:242: version_info.wValueLength < sizeof(version_info.Value) || != for this one rather ...
5 years ago (2015-11-26 21:30:00 UTC) #3
Mark Mentovai
https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_resource_reader.cc File snapshot/win/pe_image_resource_reader.cc (right): https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_resource_reader.cc#newcode141 snapshot/win/pe_image_resource_reader.cc:141: return entry.DataIsDirectory ? entry.OffsetToDirectory scottmg wrote: > The != ...
5 years ago (2015-11-27 17:56:28 UTC) #4
scottmg
https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_resource_reader.cc File snapshot/win/pe_image_resource_reader.cc (right): https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_resource_reader.cc#newcode141 snapshot/win/pe_image_resource_reader.cc:141: return entry.DataIsDirectory ? entry.OffsetToDirectory On 2015/11/27 17:56:28, Mark Mentovai ...
5 years ago (2015-11-27 18:06:59 UTC) #5
Mark Mentovai
https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_resource_reader.cc File snapshot/win/pe_image_resource_reader.cc (right): https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_resource_reader.cc#newcode141 snapshot/win/pe_image_resource_reader.cc:141: return entry.DataIsDirectory ? entry.OffsetToDirectory scottmg wrote: > Sorry, I ...
5 years ago (2015-11-27 21:40:38 UTC) #6
Mark Mentovai
Thanks for the careful review! https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_reader.cc File snapshot/win/pe_image_reader.cc (right): https://codereview.chromium.org/1475023004/diff/1/snapshot/win/pe_image_reader.cc#newcode242 snapshot/win/pe_image_reader.cc:242: version_info.wValueLength < sizeof(version_info.Value) || ...
5 years ago (2015-12-01 18:48:01 UTC) #7
scottmg
lgtm https://codereview.chromium.org/1475023004/diff/1/util/win/module_version.h File util/win/module_version.h (right): https://codereview.chromium.org/1475023004/diff/1/util/win/module_version.h#newcode27 util/win/module_version.h:27: //! This function calls `GetFileVersionInfo()`, which can implicitly ...
5 years ago (2015-12-01 19:43:02 UTC) #8
Mark Mentovai
5 years ago (2015-12-01 22:06:44 UTC) #10
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as
5be8ce4ea0fe2345ce6c843c53432e07a22cca41 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698