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.
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 email@example.com 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 <firstname.lastname@example.org>:
Author: Alan Modra <email@example.com>
Date: Fri Dec 13 16:14:57 2019 +1030
Set no file contents PT_LOAD p_offset to first page
* 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.
There's a missing range check when decoding the LDST instruction.