| Summary: | Strip leaves file offset of empty PT_LOAD segment point past end of file | ||
|---|---|---|---|
| Product: | binutils | Reporter: | Balint Reczey <balint> |
| Component: | binutils | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | amodra, i, wintheiser.durward |
| Priority: | P2 | ||
| Version: | 2.34 | ||
| Target Milestone: | --- | ||
| URL: | https://bugs.launchpad.net/ubuntu/+source/binutils/+bug/1843479 | ||
| See Also: | https://sourceware.org/bugzilla/show_bug.cgi?id=31208 | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | 2019-12-02 00:00:00 | |
| Attachments: |
Patch adjusting offset to stay in the file
Patch adjusting offset honoring page alignment Updated patch bases on mailing list comments |
||
It looks to me like your fix ignores the following comment, and will therefore cause problems with other loaders. /* We shouldn't need to align the segment on disk since the segment doesn't need file space, but the gABI arguably requires the alignment and glibc ld.so checks it. So to comply with the alignment requirement but not waste file space, we adjust p_offset for just this segment. It seemed to me that the changed adjustment later is still valid, honoring alignment. Did I miss something? (In reply to Balint Reczey from comment #2) > Did I miss something? You did. By subtracting off_adjust from the value written to p_offset you potentially have p_offset mod page_size != p_vaddr mod page_size. This will fail the glibc test in elf/dl-load.c resulting in "ELF load command address/offset not properly aligned" (In reply to Alan Modra from comment #3) > (In reply to Balint Reczey from comment #2) > > Did I miss something? > > You did. By subtracting off_adjust from the value written to p_offset you > potentially have p_offset mod page_size != p_vaddr mod page_size. > > This will fail the glibc test in elf/dl-load.c resulting in "ELF load > command address/offset not properly aligned" Thanks. Could you please consider accepting a fixed patch rather than closing the bug as won't fix? While this is not an issue with traditional loaders, users of WSL would certainly appreciate strip not generating binaries crashing there. Well, yes, but patches can be posted to binutils@sourceware.org without a bugzilla open. Created attachment 12101 [details]
Patch adjusting offset honoring page alignment
In my test with the previous patch p_offset mod page_size indeed did not match p_vaddr mod page_size. However the tested binary (gzip) worked ok, but I see that it is not guaranteed to work everywhere.
This newer patch ensures that p_offset's alignment is kept.
Created attachment 12121 [details]
Updated patch bases on mailing list comments
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0bc3450e220a4fb29f931ada84b546ce8993e85e commit 0bc3450e220a4fb29f931ada84b546ce8993e85e Author: Alan Modra <amodra@gmail.com> Date: Fri Dec 13 16:14:57 2019 +1030 Set no file contents PT_LOAD p_offset to first page PR 25237 * elf.c (assign_file_positions_for_load_sections): Attempt to keep meaningless p_offset for PT_LOAD segments without file contents within file size. Can be closed now. Closing There's a missing range check when decoding the LDST instruction. https://bubbleshooter.io/ |
Created attachment 12095 [details] Patch adjusting offset to stay in the file This itself was tolerated for long time, but WSL's ELF loader crashes due to the offset and it seems to be easy to fix.