| Age | Commit message (Collapse) | Author |
|---|
|
param.
Previously DisassemblerElfIntel<TRAITS>::ParseExecSection() passes a
hard-coded 4 to Abs32GapFinder's |abs32_width| CTOR param. This is
wrong for X64, which has abs32 pointer width of 8 bytes. This can lead
to lower quality rel32 extraction.
This CL fixes the above by replacing 4 with Traits::kVAWidth, and also
cleans up TRAITS / Traits template parameter for Disassembler:
* For template param, "template <class TRAITS>" is used throughout.
* This means function params needs to use TRAITS.
* For usage, each Disassembler class with TRAITS declares
using Traits = TRAITS;
(and variant) and uses Traits in the body of all functions. Reason:
Specialized derive classes won't have TRAITS available , so:
* Function params can use DisassemblerBase::Traits.
* Function bodies can use Traits.
* For consistency, even if TRAITS is available, still use Traits.
Bug: 1233831
Change-Id: Ie796c867fb238eca462b2fb6b4e68a965996c25a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3063919
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#908261}
NOKEYCHECK=True
GitOrigin-RevId: 294860c47cd3678c46422ce57da366724e1dc629
|
|
Previously, using Abs32GapFinder / Rel32Finder to visit gaps / rel32
references involves calling a getter that returns an optional<> value
whose emptiness indicates end of iteration. The code to use this looks
like:
for (auto value = finder.GetNext(); value; value = finder.GetNext()) {
...
}
This CL abandons optional<> usage and caches the results in Finders,
thereby removing repetition in iteration code:
while (finder.FindNext() {
auto value = finder.GetValue();
...
}
Additional changes:
* Incorporate AddressTranslator into Rel32Finder to offload translation
work from callers.
* Add tests to integrate Abs32GapFinder with Rel32Finder.
* Stylize test data to better show overlap between abs32 references
with disassembled test data.
Bug: 918867
Change-Id: Id044b67550f81c3f46ab383c5b6200906f56ca4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2918113
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#888049}
NOKEYCHECK=True
GitOrigin-RevId: 89023e1c511e599e6aeaf0b8d80e3efa2e730b5b
|
|
For PE files, rel32 scanning previously scans .text data spanning
|size_of_raw_data| bytes. However, it's possible for |virtual_size| <
|size_of_raw_data|. In this case, any rel32 references found in the
data beyond |virtual_size| would have an invalid RVA.
The problem does not cause crashes, but leads to bogus rel32 locations
to be emitted, if their bogus target happen to be valid.
This CL fixes the issue by reducing range of rel32 scan size to
|min(virtual_size, size_of_raw_data)|, thereby avoiding extracting
these invalid rel32 references.
A DCHECK is added to ParseAndStoreRel32() to validate extracted rel32
locations for {DisassemblerWin32, DisassemblerElfIntel}. This guards
against coding error (bad data should have been blocked).
Bug: 935283
Change-Id: I1587f62cb61f6cbda892b26e3d8c08cc31e0528e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1535753
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#643848}
NOKEYCHECK=True
GitOrigin-RevId: 96528abcb3e87ac56ded12f935c3443f3c96eebf
|
|
Zucchini heuristically scans assembly code byte-by-byte for rel32
references. When found, the result needs validation, and on X86 / X64,
this directs where to scan next: If accepted, scan resumes after the
instruction containing the rel32 found; if rejected, scan resumes on
the next byte.
Rel32Finder implements the above interactively: GetNext() emits the
next candidate rel32, and the caller needs to call Accept() to signal
acceptance (else rejection is assumed).
Inherited classes of Rel32Finder implements architecture-specific code
via Scan(), which caches results. Previously, Scan() also returns a
range for the instruction found. If accepted, scan resumes after the
range; if rejected, scan resumes 1 byte after the start of range.
Problem: The "scan 1 byte after" scheme works well for X86 / X64 and
fixed-size instructions (by aligning in Scan()). However, for THUMB2
instructions in ARM, which has easily discernible 2-byte and 4-byte
op codes, for both "accept" and "reject", scan should resume on the
next instruction.
This CL refactors Rel32Finder to solve the above, with other cleanup.
Details:
* Change Scan() to return (new struct) NextIterators, which stores
iterator for "accept" and "reject" cases.
* Rename Reset() to SetRegion() to assign |region_|, and remove the
|region_| assignment via constructor.
* Add Rel32FinderIntel::SetResult().
* Move more code from .h to .cc.
* Rename |next_cursor_| to |accept_it_|.
* Extensive comment updates.
Bug: 943315,918867
Change-Id: Ie0a0b380975c35b0aedb013037f8d69673c9697c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1529166
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#643098}
NOKEYCHECK=True
GitOrigin-RevId: 47fef62aa7626d9a47fc1986b8e51e6e866570d1
|
|
ClusterFuzz found a 32-bit PE file that triggers DCHECK(). The cause (in
DisassemblerWin32):
* ParseAndStoreRelocBlocks() reads malformed reloc region and rejects
it, but writes bad result to |reloc_region_| (with kInvalidOffset).
* MakeReadRelocs() passes the invalid |reloc_region_|, which causes
uint32_t overflow (for 32-bit size_t), and triggering DCHECK().
* MakeWriteRelocs() was not reached, but has similar problem.
Main fix:
* Initialize |reloc_region_| to invalid state.
* In ParseAndStoreRelocBlocks():
* On first run: Only write |reloc_region_| on success.
* On later runs: Return validity of |reloc_region_|.
* In MakeReadRelocs() and MakeWriteRelocs(): Already calls
ParseAndStoreRelocBlocks(). Change this to return vacuous
ReferenceReader / ReferenceWriter on failure, i.e., if
|reloc_region_| is invalid.
* Need to add EmptyReferenceWriter for this.
DisassemblerWin32 is also too lax in dealing with malformed input, so
this CL also implments more stringent checks:
* Disallow ImageSectionHeader (+ ImageOptionalHeader) from overlapping
with ImageDOSHeader.
* Apply more stringent checks for |size_of_optional_header| and
|number_of_rva_and_sizes|: Consistency and bounds.
Bug: 917042
Change-Id: I661303d49a57025a877585eb175f369d067652a4
Reviewed-on: https://chromium-review.googlesource.com/c/1390766
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619316}
NOKEYCHECK=True
GitOrigin-RevId: d46be686cb9e764befdd39aa168dde62b5314e37
|
|
Disassembler (Win32 and ELF) uses reloc to find abs32 locations, which
are stored in (1) |abs32_locations_|. Later on, (1) is filtered to
generate (2) actual abs32 locations.
A patching failure case was discovered: Equivalence blocks must never
cut across reference boundaries. However, it turns out blocks generated
using (2) were subject to checks using (1), which triggers a failure
since (1) - (2) is nonempty.
One way for (1) != (2) to happen is from CURRENT_MODULE() usage, which
creates an abs32 reference outside a section. This results in an abs32
target whose RVA does not map to an offset using section data, and
gets rejected by filtering logic for (2).
Fix: Apply the filtering logic direcly to (1), so (1) == (2). Details:
* Add RemoveUntranslatableAbs32() (abs32_utils.h), which uses the
filtering logic for (2) to preemptively remove problematic RVAs
from (1), so |abs32_locations_| matches (2). Extensive unit tests
are added.
* DisassemblerWin32<Traits>::ParseAndStoreAbs32(): Initialize
|abs32_locations_| with 3 steps: Naive extraction from relocs,
RemoveUntranslatableAbs32(), and RemoveOverlappingAbs32Locations().
* DisassemblerElf<Traits>::GetAbs32FromRelocSections(): Do the same,
noting that ELF's image base is always 0.
Additional fixes:
* address_translator.h: kInvalidRva was -1, but it should be -2 to
better match kInvalidOffset.
* Abs32RvaExtractorWin32::Abs32RvaExtractorWin32: The lambda
|find_and_check| binds |addr|, which has been std::move()'ed.
Better to just bind |this| and use |addr_|.
Bug: 892284
Change-Id: I628f4668ea231c7e06f35bd924652ca4d74bb848
Reviewed-on: https://chromium-review.googlesource.com/c/1263877
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598342}
NOKEYCHECK=True
GitOrigin-RevId: b6d108f1cabab2a9f3fe46a7cdeb92685a2c790e
|
|
Adds kVAWidth to Elf32Traits and use it in
DisassemblerElfIntel::MakeReferenceGroups as the width of
kReloc and kAbs32 reference types.
Bug: 892359
Change-Id: I28930b8978393c16ee29051c48496e4f696a3fcd
Reviewed-on: https://chromium-review.googlesource.com/c/1264816
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597264}
NOKEYCHECK=True
GitOrigin-RevId: af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c
|
|
Elf reloc code will go in reloc_elf.
Change-Id: Ibc93aef1cbf03a1df7b29ddf088498a70c4d3722
Reviewed-on: https://chromium-review.googlesource.com/1145688
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577557}
NOKEYCHECK=True
GitOrigin-RevId: efbbccf72cbd1778419f7e8f7c9acfd16bbb3436
|
|
depend on Disassembler.
The number of CreateEquivalenceMap() iterations used be constant
kNumIteraitons = 2. This CL makes the value depend on architecture.
Current assignment:
- DisassemblerNoOp: 1, since no pointers are identified (though in this
case, CreateEquivalenceMap() should not be called).
- DisassemblerWin32: 2.
Upcoming DisassemblerDex will use 4.
Also applying generic cleanups on headers and comments.
Bug: 729154
Change-Id: Ia12d98fcba500e4c81c8a5d356ce4cadf424ffde
Reviewed-on: https://chromium-review.googlesource.com/961273
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542919}
NOKEYCHECK=True
GitOrigin-RevId: 55aea0a875b80e614464fdd157d9717471f9d64f
|
|
(Use "git log --follow" to see older revisions of files).
/components/ is the most logical place to put Zucchini, which only
depends on /base and /testing/gtest. This move also enables Zucchini to
be used by the Component Updater. Details:
- Move all files; run the following to change deps and guards:
sed 's/chrome\/installer/components/' *.cc *.h -i
sed 's/CHROME_INSTALLER/COMPONENTS/' *.cc *.h -i
- Sorting works out pretty well!
- Change all 'chrome/installer/zucchini' to 'components/zucchini'
throughout other parts of the repo; sort if necessary.
- Fix 6 'git cl lint' errors.
- Change 1 Bind() usage to BindRepeated().
- Update OWNER.
Bug: 729154
Change-Id: I50c5a7d411ea85f707b5994ab319dfb2a1acccf7
Reviewed-on: https://chromium-review.googlesource.com/954923
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542857}
NOKEYCHECK=True
GitOrigin-RevId: 577ef6c435e8d43be6e3e60ccbcbd1881780f4ec
|
