diff --git a/ofrak_core/CHANGELOG.md b/ofrak_core/CHANGELOG.md index 76851ddc8..5048a2cac 100644 --- a/ofrak_core/CHANGELOG.md +++ b/ofrak_core/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add generic DecompilationAnalysis classes. ([#453](https://github.com/redballoonsecurity/ofrak/pull/453)) - `PatchFromSourceModifier` bundles src and header files into same temporary directory with BOM and FEM ([#517](https://github.com/redballoonsecurity/ofrak/pull/517)) - Add support for running on Windows to the `Filesystem` component. ([#521](https://github.com/redballoonsecurity/ofrak/pull/521)) +- Add new method for allocating `.bss` sections using free space ranges that aren't mapped to data ranges. ([#505](https://github.com/redballoonsecurity/ofrak/pull/505)) ### Fixed - Improved flushing of filesystem entries (including symbolic links and other types) to disk. ([#373](https://github.com/redballoonsecurity/ofrak/pull/373)) diff --git a/ofrak_core/ofrak/core/free_space.py b/ofrak_core/ofrak/core/free_space.py index 8332d1385..e1ee5ddce 100644 --- a/ofrak_core/ofrak/core/free_space.py +++ b/ofrak_core/ofrak/core/free_space.py @@ -1,7 +1,8 @@ from collections import defaultdict -from dataclasses import dataclass +from dataclasses import dataclass, field, replace from itertools import chain -from typing import List, Tuple, Dict, Optional, Iterable, Mapping +from typing import List, Tuple, Dict, Optional, Iterable, Mapping, Type +from warnings import warn from immutabledict import immutabledict @@ -31,7 +32,7 @@ class FreeSpaceAllocationError(RuntimeError): @dataclass -class FreeSpace(MemoryRegion): +class AnyFreeSpace(MemoryRegion): permissions: MemoryPermissions @index @@ -39,6 +40,16 @@ def Permissions(self) -> int: return self.permissions.value +@dataclass +class FreeSpace(AnyFreeSpace): + ... + + +@dataclass +class RuntimeFreeSpace(AnyFreeSpace): + ... + + @dataclass class FreeSpaceAllocation(ComponentConfig): permissions: MemoryPermissions @@ -58,6 +69,7 @@ class Allocatable(ResourceView): """ free_space_ranges: Dict[MemoryPermissions, List[Range]] + dataless_free_space_ranges: Dict[MemoryPermissions, List[Range]] = field(default_factory=dict) async def allocate( self, @@ -66,6 +78,7 @@ async def allocate( alignment: int = 4, min_fragment_size: Optional[int] = None, within_range: Optional[Range] = None, + with_data: bool = True, ) -> List[Range]: """ Request some range(s) of free space which satisfies the given constraints as parameters. @@ -83,6 +96,8 @@ async def allocate( :param alignment: That start of the allocated ranges will be aligned to `alignment` bytes :param min_fragment_size: The minimum size of each allocated range :param within_range: All returned ranges must be within this virtual address range + :param with_data: Whether the free space range(s) need to be mapped to some modifiable + data or not. Ranges without data are suitable for uninitialized memory e.g. `.bss`. :return: A list of one or more [Range][ofrak_type.range.Range], which each contain a start and end vaddr of an allocated range of free space @@ -96,6 +111,7 @@ async def allocate( alignment, min_fragment_size, within_range, + with_data, ) # Having acquired a satisfactory allocation, make sure subsequent calls won't allocate @@ -132,8 +148,8 @@ async def allocate_bom( for segment in obj.segment_map.values(): segments_to_allocate.append((obj, segment)) - # Allocate largest segments first - segments_to_allocate.sort(key=lambda o_s: o_s[1].length, reverse=True) + # Allocate non-bss and largest segments first + segments_to_allocate.sort(key=lambda o_s: (not o_s[1].is_bss, o_s[1].length), reverse=True) segments_by_object: Dict[str, List[Segment]] = defaultdict(list) for obj, segment in segments_to_allocate: vaddr, final_size = 0, 0 @@ -143,13 +159,15 @@ async def allocate_bom( possible_perms = permission_map[segment.access_perms] else: possible_perms = (segment.access_perms,) + alignment = max(bom.segment_alignment, segment.alignment) for candidate_permissions in possible_perms: try: allocs = await self.allocate( candidate_permissions, segment.length, min_fragment_size=segment.length, - alignment=bom.segment_alignment, + alignment=alignment, + with_data=not segment.is_bss, ) allocation = next(iter(allocs)) vaddr = allocation.start @@ -157,19 +175,30 @@ async def allocate_bom( break except FreeSpaceAllocationError: continue - if vaddr == 0 or final_size == 0: - raise FreeSpaceAllocationError( - f"Could not find enough free space for access perms {possible_perms} and " - f"length {segment.length}" - ) + + if final_size == 0: + if segment.is_bss: + # fall back to legacy .bss allocation + warn( + f"Could not find enough free space for unloaded segment with access perms " + "{possible_perms} and length {segment.length}. Assuming segment will be " + "placed with deprecated unsafe_bss_segment by ofrak_patch_maker.", + category=DeprecationWarning, + ) + vaddr = Segment.BSS_LEGACY_VADDR + final_size = segment.length + else: + raise FreeSpaceAllocationError( + f"Could not find enough free space for access perms {possible_perms} and " + f"length {segment.length}" + ) + segments_by_object[obj.path].append( - Segment( - segment_name=segment.segment_name, + replace( + segment, vm_address=vaddr, - offset=segment.offset, - is_entry=segment.is_entry, length=final_size, - access_perms=segment.access_perms, + alignment=alignment, ) ) @@ -186,10 +215,20 @@ async def _allocate( alignment: int = 4, min_fragment_size: Optional[int] = None, within_range: Optional[Range] = None, + with_data: bool = True, ) -> List[Range]: - free_ranges = self.free_space_ranges.get(permissions) - if not free_ranges: - raise FreeSpaceAllocationError(f"No free space with permissions {permissions}.") + if with_data: + free_ranges = self.free_space_ranges.get(permissions, []) + if len(free_ranges) == 0: + raise FreeSpaceAllocationError( + f"No free space with mapped data and permissions {permissions}." + ) + else: + free_ranges = self.dataless_free_space_ranges.get( + permissions, [] + ) + self.free_space_ranges.get(permissions, []) + if len(free_ranges) == 0: + raise FreeSpaceAllocationError(f"No free space with permissions {permissions}.") if min_fragment_size is None: _min_fragment_size = requested_size @@ -236,7 +275,8 @@ async def _allocate( f"units with constraints alignment={alignment}, " f"permissions={permissions}, " f"min_fragment_size={min_fragment_size}, " - f"within_range={within_range}." + f"within_range={within_range}, " + f"with_data={with_data}." ) return allocation @@ -265,12 +305,16 @@ def remove_allocation_from_cached_free_ranges( ): if len(allocation) == 0: return - new_free_ranges = remove_subranges( - self.free_space_ranges[permissions], - allocation, - ) - self.free_space_ranges[permissions] = Allocatable.sort_free_ranges(new_free_ranges) + for cache in self.free_space_ranges, self.dataless_free_space_ranges: + if permissions not in cache: + continue + + new_ranges = remove_subranges( + cache[permissions], + allocation, + ) + cache[permissions] = Allocatable.sort_free_ranges(new_ranges) class FreeSpaceAnalyzer(Analyzer[None, Allocatable]): @@ -284,22 +328,48 @@ class FreeSpaceAnalyzer(Analyzer[None, Allocatable]): targets = (Allocatable,) outputs = (Allocatable,) - async def analyze(self, resource: Resource, config: ComponentConfig = None) -> Allocatable: + @staticmethod + def _merge_ranges_by_permissions(free_spaces: Iterable[AnyFreeSpace]): ranges_by_permissions = defaultdict(list) - for free_space_r in await resource.get_descendants_as_view( - FreeSpace, - r_filter=ResourceFilter.with_tags(FreeSpace), - r_sort=ResourceSort(FreeSpace.VirtualAddress), - ): + for free_space_r in free_spaces: ranges_by_permissions[free_space_r.permissions].append(free_space_r.vaddr_range()) - merged_ranges_by_permissions = dict() + merged_ranges_by_permissions: Dict[MemoryPermissions, List[Range]] = {} for perms, ranges in ranges_by_permissions.items(): merged_ranges_by_permissions[perms] = Allocatable.sort_free_ranges( Range.merge_ranges(ranges) ) - return Allocatable(merged_ranges_by_permissions) + return merged_ranges_by_permissions + + async def analyze(self, resource: Resource, config: ComponentConfig = None) -> Allocatable: + free_spaces_with_data = [] + free_spaces_without_data = [] + + for free_space_r in await resource.get_descendants_as_view( + AnyFreeSpace, + r_filter=ResourceFilter.with_tags(AnyFreeSpace), + r_sort=ResourceSort(AnyFreeSpace.VirtualAddress), + ): + if free_space_r.resource.has_tag(RuntimeFreeSpace): + if free_space_r.resource.get_data_id() is not None: + raise ValueError( + f"Found RuntimeFreeSpace with mapped data, should be FreeSpace instead" + ) + free_spaces_without_data.append(free_space_r) + elif free_space_r.resource.has_tag(FreeSpace): + if free_space_r.resource.get_data_id() is None: + raise ValueError( + f"Found FreeSpace without mapped data, should be RuntimeFreeSpace instead" + ) + free_spaces_with_data.append(free_space_r) + else: + raise TypeError("Got AnyFreeSpace without FreeSpace or RuntimeFreeSpace tags") + + return Allocatable( + free_space_ranges=self._merge_ranges_by_permissions(free_spaces_with_data), + dataless_free_space_ranges=self._merge_ranges_by_permissions(free_spaces_without_data), + ) class RemoveFreeSpaceModifier(Modifier[FreeSpaceAllocation]): @@ -314,26 +384,26 @@ class RemoveFreeSpaceModifier(Modifier[FreeSpaceAllocation]): targets = (Allocatable,) async def modify(self, resource: Resource, config: FreeSpaceAllocation) -> None: - wholly_allocated_resources = list() - partially_allocated_resources: Dict[bytes, Tuple[FreeSpace, List[Range]]] = dict() + wholly_allocated_resources: List[AnyFreeSpace] = [] + partially_allocated_resources: Dict[bytes, Tuple[AnyFreeSpace, List[Range]]] = dict() allocatable = await resource.view_as(Allocatable) for alloc in config.allocations: for res_wholly_in_alloc in await resource.get_descendants_as_view( - FreeSpace, + AnyFreeSpace, r_filter=ResourceFilter( - tags=(FreeSpace,), + tags=(AnyFreeSpace,), attribute_filters=( ResourceAttributeValueFilter( - FreeSpace.Permissions, config.permissions.value + AnyFreeSpace.Permissions, config.permissions.value ), ResourceAttributeRangeFilter( - FreeSpace.VirtualAddress, + AnyFreeSpace.VirtualAddress, min=alloc.start, max=alloc.end - 1, ), ResourceAttributeRangeFilter( - FreeSpace.EndVaddr, min=alloc.start + 1, max=alloc.end + AnyFreeSpace.EndVaddr, min=alloc.start + 1, max=alloc.end ), ), ), @@ -360,22 +430,40 @@ async def modify(self, resource: Resource, config: FreeSpaceAllocation) -> None: for fs in wholly_allocated_resources: fs.resource.remove_tag(FreeSpace) + fs.resource.remove_tag(RuntimeFreeSpace) + fs.resource.remove_tag(AnyFreeSpace) for fs, allocated_ranges in partially_allocated_resources.values(): remaining_free_space_ranges = remove_subranges([fs.vaddr_range()], allocated_ranges) - for remaining_range in remaining_free_space_ranges: - remaining_data_range = Range.from_size( - fs.get_offset_in_self(remaining_range.start), remaining_range.length() - ) - await fs.resource.create_child_from_view( - FreeSpace( - remaining_range.start, - remaining_range.length(), - fs.permissions, - ), - data_range=remaining_data_range, - ) - fs.resource.remove_tag(FreeSpace) + if fs.resource.has_tag(FreeSpace): + for remaining_range in remaining_free_space_ranges: + remaining_data_range = Range.from_size( + fs.get_offset_in_self(remaining_range.start), remaining_range.length() + ) + await fs.resource.create_child_from_view( + FreeSpace( + remaining_range.start, + remaining_range.length(), + fs.permissions, + ), + data_range=remaining_data_range, + ) + fs.resource.remove_tag(FreeSpace) + elif fs.resource.has_tag(RuntimeFreeSpace): + for remaining_range in remaining_free_space_ranges: + await fs.resource.create_child_from_view( + RuntimeFreeSpace( + remaining_range.start, + remaining_range.length(), + fs.permissions, + ), + data_range=None, + ) + fs.resource.remove_tag(RuntimeFreeSpace) + else: + raise TypeError(f"Got AnyFreeSpace {fs} without FreeSpace or RuntimeFreeSpace tags") + + fs.resource.remove_tag(AnyFreeSpace) # Update Allocatable attributes, reflecting removed ranges allocatable.remove_allocation_from_cached_free_ranges( @@ -388,38 +476,38 @@ async def _get_partially_overlapping_resources( resource: Resource, permissions: MemoryPermissions, alloc: Range, - ) -> Iterable[FreeSpace]: + ) -> Iterable[AnyFreeSpace]: filter_overlapping_free_range_end = ( - ResourceAttributeValueFilter(FreeSpace.Permissions, permissions.value), - ResourceAttributeRangeFilter(FreeSpace.VirtualAddress, max=alloc.end), + ResourceAttributeValueFilter(AnyFreeSpace.Permissions, permissions.value), + ResourceAttributeRangeFilter(AnyFreeSpace.VirtualAddress, max=alloc.end), ResourceAttributeRangeFilter( - FreeSpace.EndVaddr, + AnyFreeSpace.EndVaddr, min=alloc.end + 1, ), ) filter_overlapping_free_range_start = ( - ResourceAttributeValueFilter(FreeSpace.Permissions, permissions.value), + ResourceAttributeValueFilter(AnyFreeSpace.Permissions, permissions.value), ResourceAttributeRangeFilter( - FreeSpace.VirtualAddress, + AnyFreeSpace.VirtualAddress, max=alloc.start - 1, ), ResourceAttributeRangeFilter( - FreeSpace.EndVaddr, + AnyFreeSpace.EndVaddr, min=alloc.start + 1, ), ) resources_overlapping_free_range_end = await resource.get_descendants_as_view( - FreeSpace, + AnyFreeSpace, r_filter=ResourceFilter( - tags=(FreeSpace,), + tags=(AnyFreeSpace,), attribute_filters=filter_overlapping_free_range_end, ), ) resources_overlapping_free_range_start = await resource.get_descendants_as_view( - FreeSpace, + AnyFreeSpace, r_filter=ResourceFilter( - tags=(FreeSpace,), + tags=(AnyFreeSpace,), attribute_filters=filter_overlapping_free_range_start, ), ) @@ -496,40 +584,63 @@ async def modify(self, resource: Resource, config: FreeSpaceModifierConfig): mem_region_view.virtual_address, mem_region_view.virtual_address + mem_region_view.size, ) - patch_data = _get_patch(freed_range, config.stub, config.fill) parent = await resource.get_parent() - patch_offset = (await resource.get_data_range_within_parent()).start - patch_range = freed_range.translate(patch_offset - freed_range.start) - # Grab tags, so they can be saved to the stub. - # At some point, it might be nice to save the attributes as well. - current_tags = resource.get_tags() + FreeSpaceTag: Type[AnyFreeSpace] + # no data within parent indicates we're marking a memory mapped region outside of the + # flash for .bss (RW) space. In theory, an ELF object could have a read only region + # of all zeros optimized out into a SHT_NOBITS section, although in practice this never + # happens and it goes into .rodata instead, so we treat a readonly config with no + # data as a user error. If you ever encounter a readonly, SHT_NOBITS section, file an issue + # to remove this check. + if resource.get_data_id() is None: + if config.permissions & MemoryPermissions.RW != MemoryPermissions.RW: + raise ValueError( + "FreeSpaceModifier on a resource with no data should only be used for RW(X) regions" + ) + + if len(config.stub) != 0: + raise ValueError("FreeSpaceModifier on a resource with no data cannot have a stub") + + freed_data_range = None + FreeSpaceTag = RuntimeFreeSpace + else: + patch_data = _get_patch(freed_range, config.stub, config.fill) + patch_offset = (await parent.get_data_range_within_parent()).start + patch_range = freed_range.translate(patch_offset - freed_range.start) + + # Patch in the patch_data + await parent.run(BinaryPatchModifier, BinaryPatchConfig(patch_offset, patch_data)) + + if len(config.stub) > 0: + # Grab tags, so they can be saved to the stub. + # At some point, it might be nice to save the attributes as well. + current_tags = resource.get_tags() + + await parent.create_child_from_view( + MemoryRegion(mem_region_view.virtual_address, len(config.stub)), + data_range=Range.from_size(patch_range.start, len(config.stub)), + additional_tags=current_tags, + ) + + freed_data_range = Range(patch_range.start + len(config.stub), patch_range.end) + FreeSpaceTag = FreeSpace + # One interesting side effect here is the Resource used to call this modifier no longer exists # when this modifier returns. This can be confusing. Would an update work better in this case? await resource.delete() await resource.save() - # Patch in the patch_data - await parent.run(BinaryPatchModifier, BinaryPatchConfig(patch_offset, patch_data)) - free_offset = len(config.stub) - if len(config.stub) > 0: - # Create the stub - await parent.create_child_from_view( - MemoryRegion(mem_region_view.virtual_address, len(config.stub)), - data_range=Range.from_size(patch_range.start, len(config.stub)), - additional_tags=current_tags, - ) - # Create the FreeSpace child await parent.create_child_from_view( - FreeSpace( + FreeSpaceTag( mem_region_view.virtual_address + free_offset, mem_region_view.size - free_offset, config.permissions, ), - data_range=Range(patch_range.start + free_offset, patch_range.end), + data_range=freed_data_range, ) diff --git a/ofrak_core/ofrak/core/patch_maker/modifiers.py b/ofrak_core/ofrak/core/patch_maker/modifiers.py index a5c0c098f..7aab04625 100644 --- a/ofrak_core/ofrak/core/patch_maker/modifiers.py +++ b/ofrak_core/ofrak/core/patch_maker/modifiers.py @@ -21,6 +21,7 @@ from ofrak.resource import Resource from ofrak.service.resource_service_i import ResourceFilter, ResourceSort, ResourceSortDirection from ofrak_type.memory_permissions import MemoryPermissions +from ofrak_type.error import NotFoundError LOGGER = logging.getLogger(__file__) @@ -189,7 +190,15 @@ def from_fem(fem: FEM) -> "SegmentInjectorModifierConfig": for segment in fem.executable.segments: if segment.length == 0: continue - segment_data = exe_data[segment.offset : segment.offset + segment.length] + + if segment.is_bss: + # It's possible for NOBITS sections like .bss to be allocated into RW MemoryRegions + # that are mapped to data. In that case, we should zero out the region instead + # of patching the arbitrary data in the FEM + segment_data = b"\0" * segment.length + else: + segment_data = exe_data[segment.offset : segment.offset + segment.length] + extracted_segments.append((segment, segment_data)) return SegmentInjectorModifierConfig(tuple(extracted_segments)) @@ -220,15 +229,14 @@ async def modify(self, resource: Resource, config: SegmentInjectorModifierConfig injection_tasks: List[Tuple[Resource, BinaryInjectorModifierConfig]] = [] for segment, segment_data in config.segments_and_data: - if segment.length == 0 or segment.vm_address == 0: + if segment.length == 0 or not segment.is_allocated: continue if segment.length > 0: LOGGER.debug( f" Segment {segment.segment_name} - {segment.length} " f"bytes @ {hex(segment.vm_address)}", ) - if segment.segment_name.startswith(".bss"): - continue + if segment.segment_name.startswith(".rela"): continue if segment.segment_name.startswith(".got"): @@ -238,18 +246,31 @@ async def modify(self, resource: Resource, config: SegmentInjectorModifierConfig # See PatchFromSourceModifier continue - patches = [(segment.vm_address, segment_data)] - region = MemoryRegion.get_mem_region_with_vaddr_from_sorted( - segment.vm_address, sorted_regions - ) - if region is None: + try: + region = MemoryRegion.get_mem_region_with_vaddr_from_sorted( + segment.vm_address, sorted_regions + ) + except NotFoundError: + # uninitialized section like .bss mapped to arbitrary memory range without corresponding + # MemoryRegion resource, no patch needed. + if segment.is_bss: + continue + raise + + region_mapped_to_data = region.resource.get_data_id() is not None + if region_mapped_to_data: + patches = [(segment.vm_address, segment_data)] + injection_tasks.append((region.resource, BinaryInjectorModifierConfig(patches))) + else: + if segment.is_bss: + # uninitialized section like .bss mapped to arbitrary memory range without corresponding + # data on a resource, no patch needed. + continue raise ValueError( f"Cannot inject patch because the memory region at vaddr " - f"{hex(segment.vm_address)} is None" + f"{hex(segment.vm_address)} is not mapped to data" ) - injection_tasks.append((region.resource, BinaryInjectorModifierConfig(patches))) - for injected_resource, injection_config in injection_tasks: result = await injected_resource.run(BinaryInjectorModifier, injection_config) # The above can patch data of any of injected_resources' descendants or ancestors diff --git a/ofrak_core/test_ofrak/components/free_space_components_test/mock_tree_struct.py b/ofrak_core/test_ofrak/components/free_space_components_test/mock_tree_struct.py index 03fdac97a..310b29ae6 100644 --- a/ofrak_core/test_ofrak/components/free_space_components_test/mock_tree_struct.py +++ b/ofrak_core/test_ofrak/components/free_space_components_test/mock_tree_struct.py @@ -3,7 +3,7 @@ from ofrak import OFRAKContext from ofrak.core.memory_region import MemoryRegion from ofrak.resource import Resource -from ofrak.core.free_space import Allocatable +from ofrak.core.free_space import Allocatable, RuntimeFreeSpace FreeSpaceTreeType = Tuple[MemoryRegion, Optional[List["FreeSpaceTreeType"]]] @@ -28,7 +28,10 @@ async def inflate_tree(tree: FreeSpaceTreeType, ofrak_context: OFRAKContext) -> async def _inflate_node(parent: MemoryRegion, node: FreeSpaceTreeType): raw_node_region, children = node - node_r = await parent.create_child_region(raw_node_region) + if isinstance(raw_node_region, RuntimeFreeSpace): + node_r = await parent.resource.create_child_from_view(raw_node_region, data_range=None) + else: + node_r = await parent.create_child_region(raw_node_region) node_r.add_view(raw_node_region) await node_r.save() if children: diff --git a/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_allocate.py b/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_allocate.py index 862a7d590..6269f8451 100644 --- a/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_allocate.py +++ b/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_allocate.py @@ -20,6 +20,7 @@ ) from ofrak_patch_maker.patch_maker import PatchMaker from ofrak_patch_maker.toolchain.model import ( + Segment, ToolchainConfig, BinFileType, CompilerOptimizationLevel, @@ -58,15 +59,24 @@ def ofrak(ofrak): @pytest.fixture def mock_allocatable(): return Allocatable( - { + free_space_ranges={ MemoryPermissions.RX: [ Range(0x100, 0x110), Range(0x80, 0xA0), Range(0xC0, 0xE0), Range(0x0, 0x40), Range(0x120, 0x200), + ], + MemoryPermissions.RW: [ + Range(0x400, 0x410), + ], + }, + dataless_free_space_ranges={ + MemoryPermissions.RW: [ + Range(0xD000, 0xD020), + Range(0xD130, 0xD200), ] - } + }, ) @@ -79,6 +89,7 @@ class AllocateTestCase: alignment: Optional[int] = 4 within_range: Optional[Range] = None mem_permissions: MemoryPermissions = MemoryPermissions.RX + with_data: bool = True ALLOCATE_TEST_CASES = [ @@ -147,7 +158,28 @@ class AllocateTestCase: "allocate with memory permissions not present", 0x100, None, - mem_permissions=MemoryPermissions.W, + mem_permissions=MemoryPermissions.RWX, + ), + AllocateTestCase( + "successful non-fragmented with_data=False allocation prefers dataless range", + 0x20, + [ + Range(0xD000, 0xD020), + ], + mem_permissions=MemoryPermissions.RW, + with_data=False, + ), + AllocateTestCase( + "successful fragmented with_data=False allocation falls back to data mapped range", + 0x100, + [ + Range(0x400, 0x410), + Range(0xD000, 0xD020), + Range(0xD130, 0xD200), + ], + mem_permissions=MemoryPermissions.RW, + min_fragment_size=0x10, + with_data=False, ), ] @@ -166,6 +198,7 @@ async def test_allocate(ofrak_context: OFRAKContext, test_case: AllocateTestCase test_case.alignment, test_case.min_fragment_size, test_case.within_range, + test_case.with_data, ) assert all([r in test_case.expected_allocation for r in alloc]) else: @@ -176,6 +209,7 @@ async def test_allocate(ofrak_context: OFRAKContext, test_case: AllocateTestCase test_case.alignment, test_case.min_fragment_size, test_case.within_range, + test_case.with_data, ) @@ -185,7 +219,8 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir): f.write( inspect.cleandoc( """ - static int global_arr[256] = {0}; + static int global_arr[64] __attribute__((section(".bss.new"))) = {0}; + static int global_arr_legacy[256] __attribute__((section(".bss.legacy"))) = {0}; int main_supplement(int a, int b) { @@ -208,7 +243,7 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir): int c = -38; int d = main_supplement(a, b) * c; (void) d; - return foo(global_arr); + return foo(global_arr_legacy); } """ @@ -230,7 +265,7 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir): no_jump_tables=True, no_bss_section=False, create_map_files=True, - compiler_optimization_level=CompilerOptimizationLevel.FULL, + compiler_optimization_level=CompilerOptimizationLevel.NONE, debug_info=True, ) @@ -253,7 +288,7 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir): resource = await ofrak_context.create_root_resource("test_allocate_bom", b"\x00") resource.add_view( Allocatable( - { + free_space_ranges={ MemoryPermissions.RX: [ Range(0x100, 0x110), Range(0x80, 0xA0), @@ -261,17 +296,21 @@ async def test_allocate_bom(ofrak_context: OFRAKContext, tmpdir): Range(0x0, 0x40), Range(0x120, 0x200), ] - } + }, + dataless_free_space_ranges={MemoryPermissions.RW: [Range(0xD000, 0xD100)]}, ) ) await resource.save() allocatable = await resource.view_as(Allocatable) - patch_config = await allocatable.allocate_bom(bom) + with pytest.warns(DeprecationWarning): + patch_config = await allocatable.allocate_bom(bom) assert len(patch_config.segments) == 1 - for segments in patch_config.segments.values(): - seg = segments[0] - assert seg.segment_name == ".text" - assert seg.vm_address == 0x120 + (segments,) = patch_config.segments.values() + segments_by_name = {seg.segment_name: seg for seg in segments} + + assert segments_by_name[".text"].vm_address == 0x120 + assert segments_by_name[".bss.new"].vm_address == 0xD000 + assert segments_by_name[".bss.legacy"].vm_address == Segment.BSS_LEGACY_VADDR diff --git a/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_analyzer.py b/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_analyzer.py index 34ed888a8..bc00c61f7 100644 --- a/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_analyzer.py +++ b/ofrak_core/test_ofrak/components/free_space_components_test/test_allocatable_analyzer.py @@ -6,7 +6,7 @@ from ofrak import OFRAKContext from ofrak.core.memory_region import MemoryRegion from ofrak.resource import Resource -from ofrak.core.free_space import Allocatable, FreeSpace +from ofrak.core.free_space import Allocatable, FreeSpace, RuntimeFreeSpace from test_ofrak.components.free_space_components_test.mock_tree_struct import ( FreeSpaceTreeType, inflate_tree, @@ -20,6 +20,7 @@ class FreeSpaceAnalyzerTestCase: label: str tree_structure: FreeSpaceTreeType expected_free_space_ranges: Dict[MemoryPermissions, List[Range]] + expected_dataless_free_space_ranges: Dict[MemoryPermissions, List[Range]] async def inflate(self, ofrak_context: OFRAKContext) -> Resource: return await inflate_tree(self.tree_structure, ofrak_context) @@ -43,6 +44,7 @@ async def inflate(self, ofrak_context: OFRAKContext) -> Resource: ], ), {MemoryPermissions.RX: [Range(0x20, 0x50)]}, + {}, ), FreeSpaceAnalyzerTestCase( "no free space", @@ -61,6 +63,7 @@ async def inflate(self, ofrak_context: OFRAKContext) -> Resource: ], ), {}, + {}, ), FreeSpaceAnalyzerTestCase( "one block of free space in multiple sibling resources", @@ -80,6 +83,7 @@ async def inflate(self, ofrak_context: OFRAKContext) -> Resource: ], ), {MemoryPermissions.RX: [Range(0x0, 0x60)]}, + {}, ), FreeSpaceAnalyzerTestCase( "one block of free space in multiple resources with weird relationships", @@ -129,6 +133,7 @@ async def inflate(self, ofrak_context: OFRAKContext) -> Resource: ], ), {MemoryPermissions.RX: [Range(0x50, 0xC8)]}, + {}, ), FreeSpaceAnalyzerTestCase( "free space ranges that would merge, but resource have different permissions", @@ -151,6 +156,29 @@ async def inflate(self, ofrak_context: OFRAKContext) -> Resource: MemoryPermissions.W: [Range(0x20, 0x50)], MemoryPermissions.X: [Range(0x50, 0x80)], }, + {}, + ), + FreeSpaceAnalyzerTestCase( + "runtime free space goes into dataless_free_space_ranges", + (MemoryRegion(0x0, 0x100), [(RuntimeFreeSpace(0x120, 0x30, MemoryPermissions.RX), None)]), + {}, + {MemoryPermissions.RX: [Range(0x120, 0x150)]}, + ), + FreeSpaceAnalyzerTestCase( + "free space ranges that would merge, but one is free space and the other is runtime free space", + ( + MemoryRegion(0x0, 0x100), + [ + (FreeSpace(0x0, 0x100, MemoryPermissions.RW), None), + (RuntimeFreeSpace(0x100, 0x30, MemoryPermissions.RW), None), + ], + ), + { + MemoryPermissions.RW: [Range(0x0, 0x100)], + }, + { + MemoryPermissions.RW: [Range(0x100, 0x130)], + }, ), ] @@ -170,6 +198,14 @@ async def test_free_space_analyzer( actual_free_ranges = allocatable.free_space_ranges[fs_type] assert actual_free_ranges == expected_free_ranges + expected_dataless_fs_types = set(test_case.expected_dataless_free_space_ranges.keys()) + actual_dataless_fs_types = set(allocatable.dataless_free_space_ranges.keys()) + assert expected_dataless_fs_types == actual_dataless_fs_types + for fs_type in expected_dataless_fs_types: + expected_dataless_free_ranges = test_case.expected_dataless_free_space_ranges[fs_type] + actual_dataless_free_ranges = allocatable.dataless_free_space_ranges[fs_type] + assert actual_dataless_free_ranges == expected_dataless_free_ranges + async def test_free_space_analysis_of_non_memory_region( ofrak_context: OFRAKContext, @@ -191,6 +227,11 @@ async def test_free_space_analysis_of_non_memory_region( FreeSpace(0x200, 0x80, MemoryPermissions.RW), data_range=Range(0x0, 0x80) ) + # child 3 is RuntimeFreeSpace with no data range + child_3 = await root_r.create_child_from_view( + RuntimeFreeSpace(0x400, 0x40, MemoryPermissions.RW), data_range=None + ) + fs_grandchild = await child_1.create_child_from_view( FreeSpace(0x1A0, 0x60, MemoryPermissions.RW), data_range=Range(0xA0, 0x100) ) @@ -198,3 +239,45 @@ async def test_free_space_analysis_of_non_memory_region( allocatable = await root_r.view_as(Allocatable) assert 1 == len(allocatable.free_space_ranges.keys()) assert [Range(0x1A0, 0x280)] == allocatable.free_space_ranges[MemoryPermissions.RW] + assert 1 == len(allocatable.dataless_free_space_ranges.keys()) + assert [Range(0x400, 0x440)] == allocatable.dataless_free_space_ranges[MemoryPermissions.RW] + + +async def test_free_space_without_data_fail( + ofrak_context: OFRAKContext, +): + # root resource is not a MemoryRegion, but it has MemoryRegion and FreeSpace descendants + root_r = await ofrak_context.create_root_resource( + "test_r", + b"\x00" * 0x1000, + ) + root_r.add_tag(Allocatable) + await root_r.save() + assert not root_r.has_tag(MemoryRegion) + + await root_r.create_child_from_view( + FreeSpace(0x100, 0x100, MemoryPermissions.RW), data_range=None + ) + + with pytest.raises(ValueError, match=r"Found FreeSpace without mapped data.*"): + await root_r.view_as(Allocatable) + + +async def test_runtime_free_space_with_data_fail( + ofrak_context: OFRAKContext, +): + # root resource is not a MemoryRegion, but it has MemoryRegion and FreeSpace descendants + root_r = await ofrak_context.create_root_resource( + "test_r", + b"\x00" * 0x1000, + ) + root_r.add_tag(Allocatable) + await root_r.save() + assert not root_r.has_tag(MemoryRegion) + + await root_r.create_child_from_view( + RuntimeFreeSpace(0x100, 0x100, MemoryPermissions.RW), data_range=Range(0x800, 0x900) + ) + + with pytest.raises(ValueError, match=r"Found RuntimeFreeSpace with mapped data.*"): + await root_r.view_as(Allocatable) diff --git a/ofrak_core/test_ofrak/components/free_space_components_test/test_allocation_modifier.py b/ofrak_core/test_ofrak/components/free_space_components_test/test_allocation_modifier.py index 6caf0ee0c..9eadd9140 100644 --- a/ofrak_core/test_ofrak/components/free_space_components_test/test_allocation_modifier.py +++ b/ofrak_core/test_ofrak/components/free_space_components_test/test_allocation_modifier.py @@ -1,5 +1,4 @@ from dataclasses import dataclass -from typing import cast import pytest @@ -7,9 +6,11 @@ from ofrak.core.memory_region import MemoryRegion from ofrak.service.resource_service_i import ResourceSort from ofrak.core.free_space import ( + AnyFreeSpace, FreeSpaceAllocation, RemoveFreeSpaceModifier, FreeSpace, + RuntimeFreeSpace, ) from test_ofrak.components.free_space_components_test.mock_tree_struct import ( FreeSpaceTreeType, @@ -30,9 +31,9 @@ class FreeSpaceAllocationModifierTestCase: async def validate_tree(actual_mem_region: MemoryRegion, expected_node: FreeSpaceTreeType): expected_mem_region, expected_children = expected_node - if isinstance(expected_mem_region, FreeSpace): - expected_free_space = cast(FreeSpace, expected_mem_region) - actual_free_space = await actual_mem_region.resource.view_as(FreeSpace) + if isinstance(expected_mem_region, AnyFreeSpace): + expected_free_space = await actual_mem_region.resource.view_as(AnyFreeSpace) + actual_free_space = await actual_mem_region.resource.view_as(AnyFreeSpace) assert ( actual_free_space == expected_free_space ), f"Got {actual_free_space} expected {expected_free_space}" @@ -95,9 +96,10 @@ async def validate_tree(actual_mem_region: MemoryRegion, expected_node: FreeSpac (FreeSpace(0x40, 0x40, MemoryPermissions.RX), None), (FreeSpace(0x80, 0x40, MemoryPermissions.RX), None), (FreeSpace(0xC0, 0x40, MemoryPermissions.RX), None), + (RuntimeFreeSpace(0xD00, 0x40, MemoryPermissions.RX), None), ], ), - FreeSpaceAllocation(MemoryPermissions.RX, [Range(0x40, 0x100)]), + FreeSpaceAllocation(MemoryPermissions.RX, [Range(0x40, 0x100), Range(0xD00, 0xD40)]), ( MemoryRegion(0x0, 0x100), [ @@ -105,6 +107,7 @@ async def validate_tree(actual_mem_region: MemoryRegion, expected_node: FreeSpac (MemoryRegion(0x40, 0x40), None), (MemoryRegion(0x80, 0x40), None), (MemoryRegion(0xC0, 0x40), None), + (MemoryRegion(0xD00, 0x40), None), ], ), ), diff --git a/ofrak_core/test_ofrak/components/test_free_space.py b/ofrak_core/test_ofrak/components/test_free_space.py index 9c73d9a67..476e2e414 100644 --- a/ofrak_core/test_ofrak/components/test_free_space.py +++ b/ofrak_core/test_ofrak/components/test_free_space.py @@ -1,3 +1,4 @@ +from ofrak.core.free_space import RuntimeFreeSpace import pytest from ofrak.core import FreeSpace, FreeSpaceModifier, FreeSpaceModifierConfig @@ -29,6 +30,19 @@ async def resource_under_test(ofrak_context: OFRAKContext) -> Resource: return memory_region +@pytest.fixture +async def dataless_resource_under_test(ofrak_context: OFRAKContext) -> Resource: + resource = await ofrak_context.create_root_resource( + "mock_empty", + b"", + (Program,), + ) + memory_region = await resource.create_child_from_view(MemoryRegion(0x0, 0x100), data_range=None) + await resource.save() + await memory_region.save() + return memory_region + + async def test_partial_free_modifier_out_of_bounds(resource_under_test: Resource): """ Test that trying to run PartialFreeSpaceModifier past a memory regions bounds results in a @@ -107,6 +121,32 @@ async def test_free_space_modifier(resource_under_test: Resource): assert child_data == config.stub +async def test_dataless_free_space_modifier(dataless_resource_under_test: Resource): + original_region = await dataless_resource_under_test.view_as(MemoryRegion) + parent = await dataless_resource_under_test.get_parent() + + rw_config = FreeSpaceModifierConfig(MemoryPermissions.RW) + await dataless_resource_under_test.run(FreeSpaceModifier, rw_config) + + # Assert runtime free space created as required + runtime_free_region = await parent.get_only_child_as_view( + MemoryRegion, r_filter=ResourceFilter.with_tags(RuntimeFreeSpace) + ) + assert original_region == runtime_free_region + + +async def test_dataless_free_space_modifier_readonly_fails(dataless_resource_under_test: Resource): + ro_config = FreeSpaceModifierConfig(MemoryPermissions.R) + with pytest.raises(ValueError, match=r".*RW.*"): + await dataless_resource_under_test.run(FreeSpaceModifier, ro_config) + + +async def test_dataless_free_space_modifier_stub_fails(dataless_resource_under_test: Resource): + stub_config = FreeSpaceModifierConfig(MemoryPermissions.RW, stub=b"\x00") + with pytest.raises(ValueError, match=r".*stub.*"): + await dataless_resource_under_test.run(FreeSpaceModifier, stub_config) + + def test_free_space_modifier_config_fill_parameters(): """ Test that the length of fill passed to `FreeSpaceModifierConfig` is greater than 0. diff --git a/ofrak_core/test_ofrak/components/test_patch_symbol_resolution.py b/ofrak_core/test_ofrak/components/test_patch_symbol_resolution.py index 6e42239cf..cf9cc6048 100644 --- a/ofrak_core/test_ofrak/components/test_patch_symbol_resolution.py +++ b/ofrak_core/test_ofrak/components/test_patch_symbol_resolution.py @@ -191,8 +191,6 @@ def symbol_test_case(request) -> TestCase: "symbol_test_case", PARAMS, indirect=["symbol_test_case"], ids=lambda tc: tc.label ) def test_symbol_resolution(patch_maker, symbol_test_case): - bss_size_required, unresolved_sym_set = patch_maker._resolve_symbols_within_BOM( - symbol_test_case.object_map - ) + unresolved_sym_set = patch_maker._resolve_symbols_within_BOM(symbol_test_case.object_map) assert unresolved_sym_set == symbol_test_case.expected_unresolved_symbols diff --git a/ofrak_patch_maker/CHANGELOG.md b/ofrak_patch_maker/CHANGELOG.md index cdc10b65c..f8ebe2230 100644 --- a/ofrak_patch_maker/CHANGELOG.md +++ b/ofrak_patch_maker/CHANGELOG.md @@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Removed `SUBALIGN(0)` for `.bss` sections - Minor update to OFRAK Community License, add OFRAK Pro License ([#478](https://github.com/redballoonsecurity/ofrak/pull/478)) - Install toolchains from source or tarball instead of relying on the Debian package manager ([#502](https://github.com/redballoonsecurity/ofrak/pull/502), [#541](https://github.com/redballoonsecurity/ofrak/pull/541)) +- Deprecate reserving `.bss` space at the FEM linking step with `create_unsafe_bss_segment` method and `unsafe_bss_segment` argument. ([#505](https://github.com/redballoonsecurity/ofrak/pull/505)) +- Determine the correct allocation behavior of sections from ELF flags rather than section names. ([#505](https://github.com/redballoonsecurity/ofrak/pull/505)) ## [4.0.2](https://github.com/redballoonsecurity/ofrak/compare/ofrak-patch-maker-v.4.0.1...ofrak-patch-maker-v.4.0.2) ### Fixed diff --git a/ofrak_patch_maker/ofrak_patch_maker/binary_parser/gnu.py b/ofrak_patch_maker/ofrak_patch_maker/binary_parser/gnu.py index b68ba0e48..b0057ce55 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/binary_parser/gnu.py +++ b/ofrak_patch_maker/ofrak_patch_maker/binary_parser/gnu.py @@ -26,7 +26,7 @@ class GNU_ELF_Parser(AbstractBinaryFileParser): r"(?P[0-9A-Fa-f]+)[ \t]+" r"(?P[0-9A-Fa-f]+)[ \t]+" r"(?P[0-9A-Fa-f]+)[ \t]+" - r"(?P[0-9]\*\*[0-9])\n[ \n]+" + r"2\*\*(?P[0-9])\n[ \n]+" r"(?P[\S, ]+)", flags=re.MULTILINE, ) @@ -62,6 +62,9 @@ def parse_sections(self, output: str) -> Tuple[Segment, ...]: permissions = permissions + MemoryPermissions.X # TODO: Figure out how to infer this. is_entry = False + + is_allocated = "ALLOC" in section_data.group("flags") + is_bss = is_allocated and "LOAD" not in section_data.group("flags") seg = Segment( segment_name=section_data.group("name"), vm_address=int(section_data.group("vma"), 16), @@ -69,6 +72,9 @@ def parse_sections(self, output: str) -> Tuple[Segment, ...]: is_entry=is_entry, length=int(section_data.group("size"), 16), access_perms=permissions, + is_allocated=is_allocated, + is_bss=is_bss, + alignment=1 << int(section_data.group("alignment")), ) segments.append(seg) return tuple(segments) diff --git a/ofrak_patch_maker/ofrak_patch_maker/binary_parser/llvm.py b/ofrak_patch_maker/ofrak_patch_maker/binary_parser/llvm.py index 96961b626..30ee3150a 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/binary_parser/llvm.py +++ b/ofrak_patch_maker/ofrak_patch_maker/binary_parser/llvm.py @@ -57,26 +57,32 @@ def _parse_readobj_section(lines: List[str], keys: Dict[str, str], flag_key: str else: kvs[remaining_keys[flag_key]] = MemoryPermissions.R + kvs["is_allocated"] = flags & 0x02 == 0x02 + del remaining_keys[flag_key] elif ": " in line: + value: Union[str, int, bool] key, value = tuple(line.strip().split(": ")) if key in remaining_keys: # Only take the value text until the first whitespace. value = value.split(" ")[0] - # Try to convert the value to an integer. - try: - value = int(value, 0) # type: ignore - except ValueError: - pass + if key == "Type": + value = value == "SHT_NOBITS" + else: + # Try to convert the value to an integer. + try: + value = int(value, 0) + except ValueError: + pass kvs[remaining_keys[key]] = value del remaining_keys[key] - if len(remaining_keys) == 0: - break + if len(remaining_keys) == 0: + break if len(remaining_keys) > 0: raise ToolchainException("Could not parse all keys!") @@ -104,6 +110,8 @@ def parse_sections(self, output: str) -> Tuple[Segment, ...]: "Offset": "offset", "Size": "length", "Flags": "access_perms", + "Type": "is_bss", + "AddressAlignment": "alignment", } return self._parse_readobj_sections(output, section_keys, "Flags") diff --git a/ofrak_patch_maker/ofrak_patch_maker/model.py b/ofrak_patch_maker/ofrak_patch_maker/model.py index 595fb1544..6e2c4f58a 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/model.py +++ b/ofrak_patch_maker/ofrak_patch_maker/model.py @@ -6,6 +6,7 @@ from dataclasses import dataclass from enum import Enum from typing import Mapping, Optional, Set, Tuple +from warnings import warn from ofrak_patch_maker.toolchain.model import Segment, BinFileType from ofrak_type.symbol_type import LinkableSymbolType @@ -34,7 +35,7 @@ class AssembledObject: :var segment_map: e.g. `{".text", Segment(...)}` :var strong_symbols: :var unresolved_symbols: {symbol name: (address, symbol type)} - :var bss_size_required: + :var bss_size_required: DEPRECATED """ path: str @@ -42,7 +43,11 @@ class AssembledObject: segment_map: Mapping[str, Segment] # segment name to Segment strong_symbols: Mapping[str, Tuple[int, LinkableSymbolType]] unresolved_symbols: Mapping[str, Tuple[int, LinkableSymbolType]] - bss_size_required: int + bss_size_required: Optional[int] = None + + def __post_init__(self): + if self.bss_size_required is not None: + warn("AssembledObject.bss_size_required is deprecated") @dataclass(frozen=True) @@ -115,17 +120,21 @@ class BOM: :var name: a name :var object_map: {source file path: AssembledObject} :var unresolved_symbols: symbols used but undefined within the BOM source files - :var bss_size_required: + :var bss_size_required: DEPRECATED :var entry_point_symbol: symbol of the patch entrypoint, when relevant """ name: str object_map: Mapping[str, AssembledObject] unresolved_symbols: Set[str] - bss_size_required: int + bss_size_required: Optional[int] entry_point_symbol: Optional[str] segment_alignment: int + def __post_init__(self): + if self.bss_size_required is not None: + warn("BOM.bss_size_required is deprecated") + class SourceFileType(Enum): DEFAULT = 0 diff --git a/ofrak_patch_maker/ofrak_patch_maker/patch_maker.py b/ofrak_patch_maker/ofrak_patch_maker/patch_maker.py index 7d764cbbc..90ce0c54a 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/patch_maker.py +++ b/ofrak_patch_maker/ofrak_patch_maker/patch_maker.py @@ -151,24 +151,17 @@ def prepare_object(self, object_path: str) -> AssembledObject: # Symbols defined in another file which may or may not be another patch source file or the target binary. relocation_symbols = self._toolchain.get_bin_file_rel_symbols(object_path) - bss_size_required = 0 segment_map = {} for s in segments: - if self._toolchain.keep_section(s.segment_name): + if self._toolchain.keep_segment(s): segment_map[s.segment_name] = s - if s.segment_name.startswith(".bss"): - if s.length > 0 and self._toolchain._config.no_bss_section: - raise PatchMakerException( - f"{s.segment_name} found but `no_bss_section` is set in the provided ToolchainConfig!" - ) - bss_size_required += s.length + return AssembledObject( object_path, self._toolchain.file_format, immutabledict(segment_map), immutabledict(symbols), immutabledict(relocation_symbols), - bss_size_required, ) @staticmethod @@ -269,15 +262,13 @@ def make_bom( object_map.update(r) # Compute the required size for the .bss segment - bss_size_required, unresolved_sym_set = self._resolve_symbols_within_BOM( - object_map, entry_point_name - ) + unresolved_sym_set = self._resolve_symbols_within_BOM(object_map, entry_point_name) return BOM( name, immutabledict(object_map), unresolved_sym_set, - bss_size_required, + None, entry_point_name, self._toolchain.segment_alignment, ) @@ -303,12 +294,10 @@ def _create_object_file( def _resolve_symbols_within_BOM( self, object_map: Dict[str, AssembledObject], entry_point_name: Optional[str] = None - ) -> Tuple[int, Set[str]]: - bss_size_required = 0 + ) -> Set[str]: symbols: Dict[str, Tuple[int, LinkableSymbolType]] = {} unresolved_symbols: Dict[str, Tuple[int, LinkableSymbolType]] = {} for o in object_map.values(): - bss_size_required += o.bss_size_required symbols.update(o.strong_symbols) # Resolve symbols defined within different patch files within the same patch BOM for sym, values in o.unresolved_symbols.items(): @@ -322,7 +311,7 @@ def _resolve_symbols_within_BOM( if entry_point_name and entry_point_name not in symbols: raise PatchMakerException(f"Entry point {entry_point_name} not found in object files") - return bss_size_required, unresolved_sym_set + return unresolved_sym_set def create_unsafe_bss_segment(self, vm_address: int, size: int) -> Segment: """ @@ -336,6 +325,13 @@ def create_unsafe_bss_segment(self, vm_address: int, size: int) -> Segment: :return: a [Segment][ofrak_patch_maker.toolchain.model.Segment] object. """ + + warn( + "Reserving .bss space at the FEM linking step is deprecated! Run " + "FreeSpaceModifier with data_range=None before allocating the BOM instead.", + category=DeprecationWarning, + ) + segment = Segment( segment_name=".bss", vm_address=vm_address, @@ -343,6 +339,7 @@ def create_unsafe_bss_segment(self, vm_address: int, size: int) -> Segment: is_entry=False, length=size, access_perms=MemoryPermissions.RW, + is_bss=True, ) align = self._toolchain.segment_alignment if vm_address % align != 0: @@ -375,7 +372,7 @@ def _build_ld( architecture-specific choices here. :param boms: BOMs and their corresponding target memory descriptions - :param bss_segment: A `.bss` segment, if any + :param bss_segment: A `.bss` segment, if any. DEPRECATED. :param additional_symbols: Additional symbols to provide to this patch, if needed :return: path to `.ld` script file @@ -389,8 +386,28 @@ def _build_ld( for segment in region_config.segments[obj.path]: # Skip the segments we're not interested in. # We have to create regions for 0-length segments to keep the linker happy! - if not self._toolchain.keep_section(segment.segment_name): + if not self._toolchain.keep_segment(segment): + continue + + if segment.vm_address == Segment.BSS_LEGACY_VADDR: + if not segment.is_bss: + raise ValueError( + "BSS_LEGACY_VADDR only supported for legacy allocation of .bss segments" + ) + + warn( + "Reserving .bss space at the FEM linking step is deprecated! Run " + "FreeSpaceModifier with data_range=None before allocating the BOM instead.", + category=DeprecationWarning, + ) + + if not bss_segment: + raise PatchMakerException( + f"BOM {bom.name} requires bss but no bss Segment allocation provided" + ) + bss_size_required += segment.length continue + memory_region, memory_region_name = self._toolchain.ld_generate_region( obj.path, segment.segment_name, @@ -400,17 +417,10 @@ def _build_ld( ) memory_regions.append(memory_region) section = self._toolchain.ld_generate_section( - obj.path, segment.segment_name, memory_region_name + obj.path, segment.segment_name, memory_region_name, segment.is_bss ) sections.append(section) - if bom.bss_size_required > 0: - if not bss_segment: - raise PatchMakerException( - f"BOM {bom.name} requires bss but no bss Segment allocation provided" - ) - bss_size_required += bom.bss_size_required - if self._toolchain.is_relocatable(): ( reloc_regions, @@ -470,7 +480,7 @@ def make_fem( :param boms: :param exec_path: - :param unsafe_bss_segment: + :param unsafe_bss_segment: DEPRECATED :param additional_symbols: :raises PatchMakerException: for invalid user inputs diff --git a/ofrak_patch_maker/ofrak_patch_maker/toolchain/abstract.py b/ofrak_patch_maker/ofrak_patch_maker/toolchain/abstract.py index ec4a970c3..0b53fc698 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/toolchain/abstract.py +++ b/ofrak_patch_maker/ofrak_patch_maker/toolchain/abstract.py @@ -80,10 +80,6 @@ def __init__( self._config = toolchain_config self._logger = logger - # The keep_list should only contain FUNCTIONALLY important sections - # (not empty .got.plt, for instance). - # TODO: Come up with a better system to handle this... - self._linker_keep_list = [".data", ".rodata", ".text", ".rel"] self._linker_discard_list = [ ".gnu.hash", ".comment", @@ -365,16 +361,8 @@ def link(self, o_files: List[str], exec_path: str, script: str = None): def linker_include_filter(symbol_name: str) -> bool: return "." in symbol_name or "_DYNAMIC" in symbol_name - def keep_section(self, section_name: str): - if section_name in self._linker_keep_list: - return True - if self._config.separate_data_sections or self._config.include_subsections: - for keep_section in self._linker_keep_list: - if section_name.startswith(keep_section): - return True - return False - else: - return False + def keep_segment(self, segment: Segment): + return segment.is_allocated and segment.segment_name not in self._linker_discard_list @abstractmethod def generate_linker_include_file(self, symbols: Mapping[str, int], out_path: str) -> str: @@ -433,7 +421,9 @@ def ld_generate_bss_region(self, vm_address: int, length: int) -> Tuple[str, str @staticmethod @abstractmethod - def ld_generate_section(object_path: str, segment_name: str, memory_region_name: str) -> str: + def ld_generate_section( + object_path: str, segment_name: str, memory_region_name: str, segment_is_bss: bool + ) -> str: """ Generates sections for linker scripts. diff --git a/ofrak_patch_maker/ofrak_patch_maker/toolchain/gnu.py b/ofrak_patch_maker/ofrak_patch_maker/toolchain/gnu.py index 93f65d752..3abf578f2 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/toolchain/gnu.py +++ b/ofrak_patch_maker/ofrak_patch_maker/toolchain/gnu.py @@ -46,6 +46,7 @@ def __init__( # if sections contain more than one function =/ "-fno-merge-constants", # avoids sections like .rodata.cst16, .rodata.str1.1 etc "-fno-reorder-functions", + "-fno-asynchronous-unwind-tables", # avoids the .eh_frame section ] ) if self._config.separate_data_sections: @@ -168,12 +169,14 @@ def ld_generate_section( object_path: str, segment_name: str, memory_region_name: str, + segment_is_bss: bool, ) -> str: stripped_seg_name = segment_name.strip(".") stripped_obj_name = os.path.basename(object_path).split(".")[0] abs_path = os.path.abspath(object_path) + noload_attr = " (NOLOAD)" if segment_is_bss else "" return ( - f" .rbs_{stripped_obj_name}_{stripped_seg_name} ORIGIN({memory_region_name}) : SUBALIGN(0) {{\n" + f" .rbs_{stripped_obj_name}_{stripped_seg_name} ORIGIN({memory_region_name}){noload_attr}: SUBALIGN(0) {{\n" f" {abs_path}({segment_name})\n" f" }} > {memory_region_name}" ) diff --git a/ofrak_patch_maker/ofrak_patch_maker/toolchain/llvm_12.py b/ofrak_patch_maker/ofrak_patch_maker/toolchain/llvm_12.py index 19f0d75f8..30c69f5f9 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/toolchain/llvm_12.py +++ b/ofrak_patch_maker/ofrak_patch_maker/toolchain/llvm_12.py @@ -49,6 +49,7 @@ def __init__( "-Xclang", "-emit-obj", "-Wall", + "-fno-asynchronous-unwind-tables", # avoids the .eh_frame section ] ) if processor.isa is InstructionSet.ARM: @@ -238,12 +239,14 @@ def ld_generate_section( object_path: str, segment_name: str, memory_region_name: str, + segment_is_bss: bool, ) -> str: stripped_seg_name = segment_name.strip(".") stripped_obj_name = os.path.basename(object_path).split(".")[0] abs_path = os.path.abspath(object_path) + noload_attr = " (NOLOAD)" if segment_is_bss else "" return ( - f" .rbs_{stripped_obj_name}_{stripped_seg_name} : {{\n" + f" .rbs_{stripped_obj_name}_{stripped_seg_name}{noload_attr} : {{\n" f" {abs_path}({segment_name}*)\n" f" }} > {memory_region_name}" ) @@ -255,7 +258,7 @@ def ld_generate_bss_section( bss_section_name = ".bss" return ( f" {bss_section_name} : {{\n" - f" *.o({bss_section_name})\n" + f" *.o({bss_section_name},{bss_section_name}.*),\n" f" }} > {memory_region_name}" ) diff --git a/ofrak_patch_maker/ofrak_patch_maker/toolchain/model.py b/ofrak_patch_maker/ofrak_patch_maker/toolchain/model.py index d61c21ed5..0d1575ebd 100644 --- a/ofrak_patch_maker/ofrak_patch_maker/toolchain/model.py +++ b/ofrak_patch_maker/ofrak_patch_maker/toolchain/model.py @@ -1,6 +1,6 @@ from dataclasses import dataclass from enum import Enum -from typing import Optional +from typing import ClassVar, Optional from ofrak_type.memory_permissions import MemoryPermissions @@ -24,14 +24,27 @@ class Segment: """ Describes a program segment. + :var segment_name: e.g. `.text` :var vm_address: where the segment is located :var offset: offset from `vm_address` :var is_entry: If the `Segment` contains the patch "entry point symbol" :var length: size of the segment in bytes :var access_perms: `rw`, `ro`, `rwx`, etc. + :var length: size of the segment in bytes + :var access_perms: `rw`, `ro`, `rwx`, etc. + :var is_allocated: True if the segment is allocated in virtual memory. Corresponds to the ELF SHF_ALLOC flag. + :var is_bss: True if the segment is a .bss sectiont that marks uninitialized data not actually present in the file. + :var alignment: Special address alignment requirement for the section + + :cvar BSS_LEGACY_VADDR: Special marker for an uninitialized section (i.e. .bss) that + is not allocated in designated free space and should be placed in a new memory region + when linking following the deprecated `unsafe_bss_segment` behavior + """ + BSS_LEGACY_VADDR: ClassVar[int] = -0xFFFFFFFF + segment_name: str vm_address: int offset: int @@ -39,6 +52,10 @@ class Segment: length: int access_perms: MemoryPermissions + is_allocated: bool = True + is_bss: bool = False + alignment: int = 1 + class CompilerOptimizationLevel(Enum): """ diff --git a/ofrak_patch_maker/ofrak_patch_maker_test/__init__.py b/ofrak_patch_maker/ofrak_patch_maker_test/__init__.py index b2d870036..ac98e50f8 100644 --- a/ofrak_patch_maker/ofrak_patch_maker_test/__init__.py +++ b/ofrak_patch_maker/ofrak_patch_maker_test/__init__.py @@ -1,6 +1,7 @@ import os from dataclasses import dataclass -from typing import Optional +from typing import Optional, Type +from ofrak_patch_maker.toolchain.abstract import Toolchain from ofrak_type import ArchInfo CURRENT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) @@ -8,7 +9,7 @@ @dataclass class ToolchainUnderTest: - toolchain: type + toolchain: Type[Toolchain] proc: ArchInfo extension: str userspace_dynamic_linker: Optional[str] = None diff --git a/ofrak_patch_maker/ofrak_patch_maker_test/example_1/hello_world.c b/ofrak_patch_maker/ofrak_patch_maker_test/example_1/hello_world.c index 7522029aa..ba3ab8e6f 100644 --- a/ofrak_patch_maker/ofrak_patch_maker_test/example_1/hello_world.c +++ b/ofrak_patch_maker/ofrak_patch_maker_test/example_1/hello_world.c @@ -1,4 +1,12 @@ -static int global_arr[256] = {0}; +#if __GNUC__ +__attribute__((section(".bss.new"))) +#endif // __GNUC__ +int global_arr[64] = {0}; + +#if __GNUC__ +__attribute__((section(".bss.legacy"))) +#endif // __GNUC__ +int global_arr_legacy[256] = {0}; int main_supplement(int a, int b) { @@ -16,10 +24,10 @@ int foo(int* arr) { __attribute__((section(".text"))) #endif // __GNUC__ int main(void) { - int a = 49; - int b = 29; - int c = -38; - int d = main_supplement(a, b) * c; - (void) d; - return foo(global_arr); + int a = 49; + int b = 29; + int c = -38; + int d = main_supplement(a, b) * c; + (void) d; + return foo(global_arr_legacy); } diff --git a/ofrak_patch_maker/ofrak_patch_maker_test/test_arm_toolchain.py b/ofrak_patch_maker/ofrak_patch_maker_test/test_arm_toolchain.py index 7c74128a8..47565467f 100644 --- a/ofrak_patch_maker/ofrak_patch_maker_test/test_arm_toolchain.py +++ b/ofrak_patch_maker/ofrak_patch_maker_test/test_arm_toolchain.py @@ -167,6 +167,7 @@ def test_arm_alignment(toolchain_under_test: ToolchainUnderTest): is_entry=False, length=0, access_perms=MemoryPermissions.RW, + is_bss=True, ) segment_dict = { patch_object.path: (text_segment_patch, data_segment_placeholder, bss_segment_placeholder), diff --git a/ofrak_patch_maker/ofrak_patch_maker_test/toolchain_c.py b/ofrak_patch_maker/ofrak_patch_maker_test/toolchain_c.py index 2e0fa9f45..c317aeeba 100644 --- a/ofrak_patch_maker/ofrak_patch_maker_test/toolchain_c.py +++ b/ofrak_patch_maker/ofrak_patch_maker_test/toolchain_c.py @@ -1,3 +1,4 @@ +from dataclasses import replace import logging import os import tempfile @@ -6,6 +7,7 @@ from ofrak_patch_maker.patch_maker import PatchMaker from ofrak_patch_maker.toolchain.gnu_avr import GNU_AVR_5_Toolchain from ofrak_patch_maker.toolchain.gnu_bcc_sparc import GNU_BCC_SPARC_Toolchain +from ofrak_patch_maker.toolchain.gnu_vbcc_m68k import VBCC_0_9_GNU_Hybrid_Toolchain from ofrak_patch_maker.toolchain.gnu_x64 import GNU_X86_64_LINUX_EABI_10_3_0_Toolchain from ofrak_patch_maker.toolchain.model import ( ToolchainConfig, @@ -112,15 +114,16 @@ def run_hello_world_test(toolchain_under_test: ToolchainUnderTest): logger = logging.getLogger("ToolchainTest") logger.setLevel("INFO") + toolchain = toolchain_under_test.toolchain(toolchain_under_test.proc, tc_config) patch_maker = PatchMaker( - toolchain=toolchain_under_test.toolchain(toolchain_under_test.proc, tc_config), + toolchain=toolchain, logger=logger, build_dir=build_dir, base_symbols=base_symbols, ) bom = patch_maker.make_bom( - name="example_3", + name="example_1", source_list=[source_path], object_list=[], header_dirs=[source_dir], @@ -131,28 +134,32 @@ def run_hello_world_test(toolchain_under_test: ToolchainUnderTest): for o in bom.object_map.values(): seg_list = [] for s in o.segment_map.values(): - seg_list.append( - Segment( - segment_name=s.segment_name, - vm_address=current_vm_address, - offset=s.offset, - is_entry=s.is_entry, - length=s.length, - access_perms=s.access_perms, - ) - ) - current_vm_address += s.length - - if toolchain_under_test.toolchain in [GNU_X86_64_LINUX_EABI_10_3_0_Toolchain]: - if current_vm_address % 16 > 0: - current_vm_address += 16 - current_vm_address % 16 + if s.segment_name == ".bss.legacy": + # test legacy allocation of .bss + seg_list.append(replace(s, vm_address=Segment.BSS_LEGACY_VADDR)) + # Set the unsafe .bss size to exactly this length, the other .bss section + # will be allocated the normal way + unsafe_bss_size = s.length else: - if current_vm_address % 4 > 0: - current_vm_address += 4 - current_vm_address % 4 + seg_list.append(replace(s, vm_address=current_vm_address)) + current_vm_address += s.length + + if toolchain_under_test.toolchain in [GNU_X86_64_LINUX_EABI_10_3_0_Toolchain]: + if current_vm_address % 16 > 0: + current_vm_address += 16 - current_vm_address % 16 + else: + if current_vm_address % 4 > 0: + current_vm_address += 4 - current_vm_address % 4 all_segments.update({o.path: tuple(seg_list)}) - bss = patch_maker.create_unsafe_bss_segment(current_vm_address, 0x8000) + # TODO: can we get VBCC/GNU hybrid toolchain to emit separate .bss sections for testing? + if toolchain_under_test.toolchain == VBCC_0_9_GNU_Hybrid_Toolchain: + unsafe_bss_size = 8000 + else: + current_vm_address += 16 - current_vm_address % 16 # align unsafe .bss segment + + bss = patch_maker.create_unsafe_bss_segment(current_vm_address, unsafe_bss_size) allocator_config = PatchRegionConfig(bom.name + "_patch", all_segments) diff --git a/ofrak_tutorial/notebooks_with_outputs/6_code_insertion_with_extension.ipynb b/ofrak_tutorial/notebooks_with_outputs/6_code_insertion_with_extension.ipynb index 051d4202b..b65cf13d8 100644 --- a/ofrak_tutorial/notebooks_with_outputs/6_code_insertion_with_extension.ipynb +++ b/ofrak_tutorial/notebooks_with_outputs/6_code_insertion_with_extension.ipynb @@ -704,10 +704,10 @@ "text": [ "BOM dataclass:\n", "name: hello_world_patch\n", - "bss_size_required: 0\n", + "bss_size_required: None\n", "entry_point_symbol (optional): None\n", "\n", - "object map[c_patch]: AssembledObject(path='/tmp/tmp10mi8gpg/hello_world_patch_bom_files/c_patch.c.o', file_format=, segment_map=immutabledict({'.text': Segment(segment_name='.text', vm_address=0, offset=64, is_entry=False, length=70, access_perms=)}), strong_symbols=immutabledict({'c_patch.c': (0, ), 'uppercase_and_print': (0, )}), unresolved_symbols=immutabledict({'puts': (0, )}), bss_size_required=0)\n", + "object map[c_patch]: AssembledObject(path='/tmp/tmp10mi8gpg/hello_world_patch_bom_files/c_patch.c.o', file_format=, segment_map=immutabledict({'.text': Segment(segment_name='.text', vm_address=0, offset=64, is_entry=False, length=70, access_perms=, is_allocated=True, is_bss=False, alignment=4)}), strong_symbols=immutabledict({'c_patch.c': (0, ), 'uppercase_and_print': (0, )}), unresolved_symbols=immutabledict({'puts': (0, )}), bss_size_required=None)\n", "\n" ] }