Age | Commit message (Collapse) | Author |
|
Use template read/write functions instead of repeated versions.
Change-Id: Ie87d307ebd7b297fe802216fe07aa820d7b1fa4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3082253
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918237}
NOKEYCHECK=True
GitOrigin-RevId: d64aec31e8bb5e1acb9a2da1e6e92fbd5e59d5f6
|
|
This CL enables ARM-ELF (AArch32 and AArch64) support in Zucchini.
* Define ARM {AArch32, AArch64}ReferenceType.
* Add Rel32Finder{Arm, AArch32, AArch64} (with tests) to use
previously-added ARM disassembly code to extract rel32 references.
* Add DisassemblerElf{Arm, AArch32, AArch64} to parse ARM ELF files and
create reference readers / writers, and reference groups.
* For AArch32: Add heuristic detection of ARM vs. Thumb2 mode.
* Add IsTargetOffsetInElfSectionList() (with tests) to help ARM reject
false positive references.
* Add ReferenceBytesMixerElfArm to remove redundant reference target
information from bytewise correction data.
Bug: 918867
Change-Id: I1e6d3d8b8d174c85a3d44ca6d642b7ff0bd6a6a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2922822
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#908913}
NOKEYCHECK=True
GitOrigin-RevId: 85cc8a596f183487b395a59e80b2f654f241ab2c
|
|
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
|
|
This CL was uploaded by git cl split.
R=wfh@chromium.org
Bug: 1216696
Change-Id: I78d558e20d5e4056b4470ff6a9b9395f72a61631
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2975779
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#894795}
NOKEYCHECK=True
GitOrigin-RevId: 3a9b13d917c7dc2de170fdbd22fa19ac376daa8d
|
|
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
|
|
This fixes a case where 32-bit Zucchini with size being 32-bit will
crash when handling a 64-bit ELF file if there is a segment that
extends beyond the image bounds.
This is a quick fix, but there is a larger issue at play here. See the
bug to discuss options.
Bug: 1035603
Change-Id: I74887b8c6b8779642910de1bc31dccfcdc0d1bec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008356
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733625}
NOKEYCHECK=True
GitOrigin-RevId: f3ba433da667d8cda30ac4db8402a9137c281ee1
|
|
The fuzzer for the disassembler_elf found a couple of ways to trigger
checked_cast failures in the ParseHeader function. Other disassemblers
handle such failures by cleanly exiting as opposed to crashing due to a
CHECK. This turned out to be a problem with numeric overflow in
JudgeSection.
Bug: 1029405
Change-Id: Idae395f74a43a1de4793db6222d7786e57e9ad30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1967070
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#725225}
NOKEYCHECK=True
GitOrigin-RevId: 925bb161e0dcd816510f616190a2ba24c0bea2bb
|
|
This CL redoes section processing in DisassemblerElf to fix 4 bugs
discovered by zucchini_disassembler_elf_fuzzer that ckitagawa@ recently
added. These bugs all involve some malformed SHF_EXECINSTR section that
sneaks past early checks to reach pointer extraction, then triggers
another check. Summaries:
* Issue 1023095: |sh_type == SHT_NOBITS| helps section bypass bound
checks. DCHECK is triggered in Abs32GapFinder::Abs32GapFinder()
because |sh_size| grossly exceeds bounds.
* Issue 1023183: |sh_size == 0| and |sh_offset < image_.size()| makes
section seem benign, but |sh_offset| is ignored when computing
|offset_bound|. DCHECK is triggred in Abs32GapFinder() ctor because
|sh_offset| > estimated image size (|offset_bound|).
* Issue 1023203: ELF64. |sh_size == 0| and |sh_offset == 0| make
section seem benign. However, |sh_addr| far exceeds 32-bit bound. In
ParseExecSection(), |sh_addr| fails base::checked_cast().
* Issue 1023210: Section has vaid bounds, and |sh_addr == 0| makes
section excluded (heuristically) from AddressTranslator. Section
proceeds to ParseExecSection(), which finds a rel32 whose:
* Location offset is assumed okay.
* Location RVA, by optimization, is converted directly using section
data, and is also okay.
* Target RVA is validated by AddressTranslator.
But in Rel32ReaderX86::GetNext(), location offset -> RVA now uses
AddressTranslator, which by earlier exclusion, results in
kInvalidOffset. This pollutes target RVA and target offset, and
triggers DCHECK.
The above shows mismatches among usage of sections for the following:
* Location / RVA matching (AddressTranslator),
* ELF image size estimation (|offset_bound|),
* Pointer extraction,
against bypasses due to |sh_type == SHT_NOBITS|, |sh_size == 0|, and
|sh_addr == 0|.
To fix the issues, this CL separates decision logic from enactment.
Decision logic is moved to JudgeSections(), which takes a section and
returns a "judgement" consisting of bit field defined from new enum
SectionJudgement. The judgement is enacted in ParseHeader(), which
chooses to invalidate the ELF, ignore the section, or use the section
with greater discretion on applying pointer extraction.
Additional fix: Ignore (non-fatal) sections with SHF_TLS bit, since
these sections can have offset-RVA ranges that conflict with other
section's. Without this fix, Zucchini on Ubuntu won't recognize itself
as an ELF file!
Bug: 1023095, 1023183, 1023203, 1023210, 1022538
Change-Id: Icc86f26db17a61bb41b432177ef6c3dc0dcd1a26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1933632
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718809}
NOKEYCHECK=True
GitOrigin-RevId: f2e5fba3a476a96d5a412603385cf47116c51251
|
|
The fuzzer found a pathological case when the section size is 0 but the
offset is outside of image. This resulted in header parsing skipping
the section since the size was 0; however, later processing creates a
region of size 0 that is outside the image causing checks to fail. The
solution here is to check if the offset is outside the image and the
size is 0. This suggests that the data is ill formed and we should
reject the image entirely.
Bug: 1019271
Change-Id: If47d099aa4f919b097d4e15804048eaf64a59201
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903886
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713572}
NOKEYCHECK=True
GitOrigin-RevId: 74eb15eee824427077620f88b2e4759c5bb2e221
|
|
The current code is too lax. It doesn't enforce bounds checks
strongly enough. It claims to be for RVAs, but allows all sections
through. This results in downstream code being unable to trust that the
regions created are safely within the image resulting in issues when
Fuzzing if the data is ill formed. To fix the fuzzers we should be
remove this forgiveness. However, long term a better check for RVA
forgiveness should maybe be investigated.
Bug: 1013823, 1013842, 1013871, 1014124
Change-Id: Ic164fc76d687711c496f57b3bfe33ced6b8ad838
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863070
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706511}
NOKEYCHECK=True
GitOrigin-RevId: 73089e0c2f9bc4c901c2e86e3d498e40dccb8172
|
|
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
|
|
Update includes and comments, and remove some unused code.
In particular, remove TODO comments for figuring out whether ARM
abs32 references can be 4 bytes long: Turns out ARM absolute
references are 8 bytes long. It's rel32 refereneces that can be
4 bytes long.
Change-Id: I02dc905885f6cb5ff929efe0fb1f9a6593ee05a8
Reviewed-on: https://chromium-review.googlesource.com/c/1327559
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@{#606612}
NOKEYCHECK=True
GitOrigin-RevId: 9076fc4939ced233b85e5f8942ba947b6143aba3
|
|
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
|
|
Fix compile error with -Wshorten-64-to-32.
Bug: 881008
Change-Id: I52a1bab9cb7b4cb67ea4c275143390d42b192120
Reviewed-on: https://chromium-review.googlesource.com/1216524
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590285}
NOKEYCHECK=True
GitOrigin-RevId: f6c70a2cc49b3a1765aa38763532e948c48a65a1
|
|
Creates Disassembler that recognises and parses ELF format. For now, it only supports Intel architeture. Support for Arm will be added in follow-up CLs.
Change-Id: Ibdcf113b573f22844b6a1611c5ff6df46829b9b3
Reviewed-on: https://chromium-review.googlesource.com/1136841
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582233}
NOKEYCHECK=True
GitOrigin-RevId: 3c64e078fea9f23e44939c25ca02cf05b72b2c40
|