From 2d26ace5e5baaa29587187f760a4163f359a925d Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 12 Apr 2023 18:18:30 +1000 Subject: [PATCH 1/4] elf: Fix for mismatched app ELF file not detected. The check that the app ELF file SHA256 matches the one stored in the core dump would never fail, leading to gdb loading the wrong ELF file and either crashing or producing misleading debug information. Specifics: The note_sec.name field was incorrectly read back as b'ESP_CORE_DUMP_INFO\x00E', because the namesz length includes the terminating NUL byte and possible junk padding bytes: https://github.com/espressif/esp-idf/blob/master/components/espcoredump/src/core_dump_elf.c#L212 In addition, as 'note_sec.name' is a bytes object Python 3 would have never successfully compared it with a string. --- components/espcoredump/corefile/elf.py | 7 +++++++ components/espcoredump/corefile/loader.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/components/espcoredump/corefile/elf.py b/components/espcoredump/corefile/elf.py index 8ffa12b6cd..76c2c60a63 100644 --- a/components/espcoredump/corefile/elf.py +++ b/components/espcoredump/corefile/elf.py @@ -284,6 +284,13 @@ class ElfNoteSegment(ElfSegment): super(ElfNoteSegment, self).__init__(addr, data, flags) self.type = ElfFile.PT_NOTE self.note_secs = NoteSections.parse(self.data) + for note in self.note_secs: + # note.name should include a terminating NUL byte, plus possible + # padding + # + # (note: construct.PaddingString can't parse this if there + # are non-zero padding bytes after the NUL, it also parses those.) + note.name = note.name.split(b'\x00')[0] @staticmethod def _type_str(): # type: () -> str diff --git a/components/espcoredump/corefile/loader.py b/components/espcoredump/corefile/loader.py index 144a350826..8f9fce7442 100644 --- a/components/espcoredump/corefile/loader.py +++ b/components/espcoredump/corefile/loader.py @@ -261,7 +261,7 @@ class EspCoreDumpLoader(EspCoreDumpVersion): for seg in core_elf.note_segments: for note_sec in seg.note_secs: # Check for version info note - if note_sec.name == 'ESP_CORE_DUMP_INFO' \ + if note_sec.name == b'ESP_CORE_DUMP_INFO' \ and note_sec.type == ESPCoreDumpElfFile.PT_INFO \ and exe_name: exe_elf = ElfFile(exe_name) From 76e1212c8f61328ae6de29cfe4d2595c55b5e4eb Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 13 Apr 2023 11:43:06 +1000 Subject: [PATCH 2/4] elf: Fix SHA256 calculation The comment says it returns the "SHA256 hash of the input ELF file", but this is not true - it was the SHA256 hash of the output ELF file. As the parser may change some bytes around in minor ways, these were often not the same. --- components/espcoredump/corefile/elf.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/components/espcoredump/corefile/elf.py b/components/espcoredump/corefile/elf.py index 76c2c60a63..4ce1eabf8d 100644 --- a/components/espcoredump/corefile/elf.py +++ b/components/espcoredump/corefile/elf.py @@ -126,6 +126,8 @@ class ElfFile(object): self.load_segments = [] # type: list[ElfSegment] self.note_segments = [] # type: list[ElfNoteSegment] + self.sha256 = b'' # type: bytes + if elf_path and os.path.isfile(elf_path): self.read_elf(elf_path) @@ -156,6 +158,13 @@ class ElfFile(object): sec.data, sec.sh.sh_flags) for sec in self._model.sections] + # calculate sha256 of the input bytes (note: this may not be the same as the sha256 of any generated + # output struct, as the ELF parser may change some details.) + sha256 = hashlib.sha256() + sha256.update(elf_bytes) + self.sha256 = sha256.digest() + + @staticmethod def _parse_string_table(byte_str, offset): # type: (bytes, int) -> str section_name_str = byte_str[offset:] @@ -215,15 +224,6 @@ class ElfFile(object): return Struct(*args) - @property - def sha256(self): # type: () -> bytes - """ - :return: SHA256 hash of the input ELF file - """ - sha256 = hashlib.sha256() - sha256.update(self._struct.build(self._model)) # type: ignore - return sha256.digest() - class ElfSection(object): SHF_WRITE = 0x01 From e32cca2ad170edd6962daabd7c891fba41c34fcf Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 13 Apr 2023 11:44:01 +1000 Subject: [PATCH 3/4] loader: Fix handling of APP_RETRIEVE_LEN_ELF_SHA With the default APP_RETRIEVE_LEN_ELF_SHA setting, core dump files only have a truncated ELF SHA256 in them. Account for this when comparing the core dump SHA with the app ELF SHA. --- components/espcoredump/corefile/loader.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/components/espcoredump/corefile/loader.py b/components/espcoredump/corefile/loader.py index 8f9fce7442..880c974eff 100644 --- a/components/espcoredump/corefile/loader.py +++ b/components/espcoredump/corefile/loader.py @@ -271,10 +271,20 @@ class EspCoreDumpLoader(EspCoreDumpVersion): 'sha256' / Bytes(64) # SHA256 as hex string ) coredump_sha256 = coredump_sha256_struct.parse(note_sec.desc[:coredump_sha256_struct.sizeof()]) - if coredump_sha256.sha256 != app_sha256: + + logging.debug('App SHA256: {!r}'.format(app_sha256)) + logging.debug('Core dump SHA256: {!r}'.format(coredump_sha256)) + + # Actual coredump SHA may be shorter than a full SHA256 hash + # with NUL byte padding, according to the app's APP_RETRIEVE_LEN_ELF_SHA + # length + core_sha_trimmed = coredump_sha256.sha256.rstrip(b'\x00').decode() + app_sha_trimmed = app_sha256[:len(core_sha_trimmed)].decode() + + if core_sha_trimmed != app_sha_trimmed: raise ESPCoreDumpLoaderError( - 'Invalid application image for coredump: coredump SHA256({!r}) != app SHA256({!r}).' - .format(coredump_sha256, app_sha256)) + 'Invalid application image for coredump: coredump SHA256({}) != app SHA256({}).' + .format(core_sha_trimmed, app_sha_trimmed)) if coredump_sha256.ver != self.version: raise ESPCoreDumpLoaderError( 'Invalid application image for coredump: coredump SHA256 version({}) != app SHA256 version({}).' From e13e3bff7e1dde7804d3becd4f5aa8f151be84b7 Mon Sep 17 00:00:00 2001 From: Roland Dobai Date: Fri, 14 Apr 2023 13:09:57 +0200 Subject: [PATCH 4/4] espcoredump: Fix Python style --- components/espcoredump/corefile/elf.py | 1 - components/espcoredump/corefile/loader.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/components/espcoredump/corefile/elf.py b/components/espcoredump/corefile/elf.py index 4ce1eabf8d..82527cf72e 100644 --- a/components/espcoredump/corefile/elf.py +++ b/components/espcoredump/corefile/elf.py @@ -164,7 +164,6 @@ class ElfFile(object): sha256.update(elf_bytes) self.sha256 = sha256.digest() - @staticmethod def _parse_string_table(byte_str, offset): # type: (bytes, int) -> str section_name_str = byte_str[offset:] diff --git a/components/espcoredump/corefile/loader.py b/components/espcoredump/corefile/loader.py index 880c974eff..dbf292d517 100644 --- a/components/espcoredump/corefile/loader.py +++ b/components/espcoredump/corefile/loader.py @@ -88,7 +88,7 @@ class EspCoreDumpVersion(object): COREDUMP_SUPPORTED_TARGETS = XTENSA_CHIPS + RISCV_CHIPS - def __init__(self, version=None): # type: (int) -> None + def __init__(self, version=None): # type: (Optional[int]) -> None """Constructor for core dump version """ super(EspCoreDumpVersion, self).__init__() @@ -248,7 +248,7 @@ class EspCoreDumpLoader(EspCoreDumpVersion): else: raise NotImplementedError - def _extract_elf_corefile(self, exe_name=None, e_machine=ESPCoreDumpElfFile.EM_XTENSA): # type: (str, int) -> None + def _extract_elf_corefile(self, exe_name=None, e_machine=ESPCoreDumpElfFile.EM_XTENSA): # type: (Optional[str], int) -> None """ Reads the ELF formatted core dump image and parse it """