Skip to content

Commit

Permalink
elf2flt: fix for segfault on some ARM ELFs
Browse files Browse the repository at this point in the history
I believe a bug was introduced in commit [1], which was done to move the .ARM.exidx input section from .data to .text output section.  However, in doing so, the dynamic memory allocation for .text [elf2flt.c:~L1907: 'text = xmalloc(text_len);'] was not modified to allow for the additional .ARM.exidx section.  The result was that most of the time malloc() allocated enough extra memory [due to page-size or similar allocation boundaries] such that a small additional section went unnoticed.  However, in unlucky cases, the memory required for just the .text section fit almost perfectly within an allocation boundary, and the extra .ARM.exidx section exceeded the allocation, and thus produced a segfault when the memory was written.

The fix here modifies the calculation of 'text_len' in main() with logic similar to that of the original fix in [1] to output_relocs(), such that the correct amount of memory is allocated.  The code is also modified such that 'data_len' is also calculated correctly, and does not over-allocate memory.  This is necessary because the logic in main() is not a single grouping of if-elseif... priority logic as in output_relocs().

The change also attempts to be even more specific with input section selections for .text and .data output sections.
.text is only selected for input sections flagged as (SEC_CODE || (SEC_DATA && SEC_READONLY && SEC_RELOC))
.data is only selected for input sections flagged as (SEC_DATA && !(SEC_READONLY && SEC_RELOC))

The change appears to work correctly for previously segfault-causing ELF with these processed sections...
SEC_FLAGS:0x0000011f SEC_NAME:.text
SEC_FLAGS:0x0000012f SEC_NAME:.ARM.exidx
SEC_FLAGS:0x00000127 SEC_NAME:.data
SEC_FLAGS:0x00000123 SEC_NAME:.tm_clone_table
SEC_FLAGS:0x0000012b SEC_NAME:.eh_frame
SEC_FLAGS:0x00000001 SEC_NAME:.bss
SEC_FLAGS:0x00000100 SEC_NAME:.stack
SEC_FLAGS:0x01800108 SEC_NAME:.comment
SEC_FLAGS:0x00000108 SEC_NAME:.ARM.attributes
SEC_FLAGS:0x0000210c SEC_NAME:.debug_aranges
SEC_FLAGS:0x0000210c SEC_NAME:.debug_info
SEC_FLAGS:0x00002108 SEC_NAME:.debug_abbrev
SEC_FLAGS:0x0000210c SEC_NAME:.debug_line
SEC_FLAGS:0x0000210c SEC_NAME:.debug_frame
SEC_FLAGS:0x01802108 SEC_NAME:.debug_str
SEC_FLAGS:0x0000210c SEC_NAME:.debug_loc
SEC_FLAGS:0x00002108 SEC_NAME:.debug_ranges

It should also be pointed out that this commit may impact the regression noted in [2] and pull-request [3].
With this code change in place, elf2flt will put section .eh_frame with flags=0x12b in the .data output section.  If the flags for an .eh_frame section were 0x12f (same as .ARM.exidx), then it would end up in .text output section.

A few cosmetic changes are included as well.

[1] 73325b7
[2] #12
[3] #16

Signed-off-by: Mike Pilawa <Mike.Pilawa@csiro.au>
  • Loading branch information
mpilawa committed Aug 19, 2021
1 parent ebec578 commit 79d3e86
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions elf2flt.c
Expand Up @@ -363,7 +363,7 @@ output_relocs (
#endif

#if 0
printf("%s(%d): output_relocs(abs_bfd=%d,synbols=0x%x,number_of_symbols=%d"
printf("%s(%d): output_relocs(abs_bfd=%d,symbols=0x%x,number_of_symbols=%d,"
"n_relocs=0x%x,text=0x%x,text_len=%d,data=0x%x,data_len=%d)\n",
__FILE__, __LINE__, abs_bfd, symbols, number_of_symbols, n_relocs,
text, text_len, data, data_len);
Expand Down Expand Up @@ -424,7 +424,8 @@ output_relocs (
*/
if ((!pic_with_got || ALWAYS_RELOC_TEXT) &&
((a->flags & SEC_CODE) ||
((a->flags & (SEC_DATA | SEC_READONLY)) == (SEC_DATA | SEC_READONLY))))
((a->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
(SEC_DATA | SEC_READONLY | SEC_RELOC))))
sectionp = text + (a->vma - text_vma);
else if (a->flags & SEC_DATA)
sectionp = data + (a->vma - data_vma);
Expand Down Expand Up @@ -1875,7 +1876,9 @@ int main(int argc, char *argv[])
bfd_size_type sec_size;
bfd_vma sec_vma;

if (s->flags & SEC_CODE) {
if ((s->flags & SEC_CODE) ||
((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
(SEC_DATA | SEC_READONLY | SEC_RELOC))) {
vma = &text_vma;
len = &text_len;
} else if (s->flags & SEC_DATA) {
Expand Down Expand Up @@ -1908,9 +1911,13 @@ int main(int argc, char *argv[])
if (verbose)
printf("TEXT -> vma=0x%x len=0x%x\n", text_vma, text_len);

/* Read in all text sections. */
/* Read input sections destined for the text output segment.
* Includes code sections, but also includes read-only relocation
* data sections.*/
for (s = abs_bfd->sections; s != NULL; s = s->next)
if (s->flags & SEC_CODE)
if ((s->flags & SEC_CODE) ||
((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
(SEC_DATA | SEC_READONLY | SEC_RELOC)))
if (!bfd_get_section_contents(abs_bfd, s,
text + (s->vma - text_vma), 0,
bfd_section_size(abs_bfd, s)))
Expand All @@ -1934,9 +1941,13 @@ int main(int argc, char *argv[])
text_len = data_vma - text_vma;
}

/* Read in all data sections. */
/* Read input sections destined for the data output segment.
* Includes data sections, but not those read-only relocation
* data sections already included in the text output section.*/
for (s = abs_bfd->sections; s != NULL; s = s->next)
if (s->flags & SEC_DATA)
if ((s->flags & SEC_DATA) &&
((s->flags & (SEC_READONLY | SEC_RELOC)) !=
(SEC_READONLY | SEC_RELOC)))
if (!bfd_get_section_contents(abs_bfd, s,
data + (s->vma - data_vma), 0,
bfd_section_size(abs_bfd, s)))
Expand Down

0 comments on commit 79d3e86

Please sign in to comment.