Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elf2flt: fix for segfault on some ARM ELFs #20

Merged
merged 1 commit into from Aug 19, 2021

Conversation

mpilawa
Copy link
Contributor

@mpilawa mpilawa commented Nov 8, 2020

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

@vapier vapier changed the base branch from master to main January 13, 2021 22:48
elf2flt.c Outdated Show resolved Hide resolved
elf2flt.c Show resolved Hide resolved
@vapier
Copy link
Member

vapier commented Aug 19, 2021

latest version looks great, but you'll need to rebase onto the latest branch. can you pull+rebase so i can merge ?

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] uclinux-dev@73325b7
[2] uclinux-dev#12
[3] uclinux-dev#16

Signed-off-by: Mike Pilawa <Mike.Pilawa@csiro.au>
@mpilawa
Copy link
Contributor Author

mpilawa commented Aug 19, 2021

Should be all in sync and good to go now.

@vapier vapier merged commit ba379d0 into uclinux-dev:main Aug 19, 2021
@vapier
Copy link
Member

vapier commented Aug 19, 2021

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants