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

Fixes for MIPS binaries #180

Merged
merged 2 commits into from Aug 11, 2021
Merged

Fixes for MIPS binaries #180

merged 2 commits into from Aug 11, 2021

Conversation

iv-m
Copy link
Contributor

@iv-m iv-m commented Aug 22, 2019

A couple of changes that address problems described in #82.

Tested on two machines, Baikal BE-T1000 and Loongson 3A, running mipsel port of ALT Linux Sisyphus. With these two changes, all tests pass on both of the machines, and on my x86_64 machine.

@iv-m
Copy link
Contributor Author

iv-m commented Aug 22, 2019

UPD: replaced ptrdiff_t with auto to make gcc5 happy (I was testing with gcc8).

@olekw
Copy link

olekw commented Apr 5, 2020

@edolstra: are you ok with this pull request? Unavailability of patchelf on MIPS is leading to a bunch of issues downstream in Debian. To expedite a fix, we could take this patch and implement it downstream until a newer version of patchelf is released. However, I'm hesitant to do that if you have a fundamental disagreement with how this solves #82. Thanks!

@domenkozar
Copy link
Member

@darealshinji would you review/test this?

@fsateler
Copy link
Contributor

I can confirm this makes tests pass on debian mipsel, on a Rhino Labs UTM8

@domenkozar
Copy link
Member

It would be great if someone stepped up to be maintainer of MIPS as per #199

@iv-m
Copy link
Contributor Author

iv-m commented Jun 21, 2021

UPD: rebased onto the current master with a couple of minor cosmetic changes.

src/patchelf.cc Outdated
else if (d_tag == DT_MIPS_RLD_MAP_REL) {
/* the MIPS_RLD_MAP_REL tag stores the offset to the debug
pointer, relative to the address of the tag */
debug("Updating DT_MIPS_RLD_MAP_REL");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick. we don't do any debug printing for other sections:

Suggested change
debug("Updating DT_MIPS_RLD_MAP_REL");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm going to drop this.

@@ -1129,6 +1141,14 @@ void ElfFile<ElfFileParamNames>::rewriteHeaders(Elf_Addr phdrAddress)
dyn->d_un.d_ptr = findSection(".gnu.version_r").sh_addr;
else if (d_tag == DT_VERSYM)
dyn->d_un.d_ptr = findSection(".gnu.version").sh_addr;
else if (d_tag == DT_MIPS_RLD_MAP_REL) {
Copy link
Member

@Mic92 Mic92 Aug 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the .rld_map section be stripped just like DT_JMPREL or DT_REL?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I am asking is, is it possible to that someone has binaries where this section was stripped.
I fixed a similar issue here: #296

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mic92, thank you for the review.

I understood what you're asking, and I need to give it some thought and run a couple of experiments.

I used to believe that elf with DT_MIPS_RLD_MAP_REL in dynamic section but without .rld_map is simply broken, but maybe patchelf needs to handle such files. If that's the case, I'll update the code here and add a test for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, elf file with DT_MIPS_RLD_MAP_REL in dynamic section but without .rld_map is indeed broken: dynamic loader (I've checked glibc 2.32) just writes the debug pointer to the address it calculates from DT_MIPS_RLD_MAP_REL entry. I guess no tool produces such binaries.

But when I just removed .rtld_map section with objcopy, the resulting binaries mostly worked -- out of sheer luck probably, but still most of them did not crash.

So I rewrote the code to handle such case. Removing DT_MIPS_RLD_MAP_REL might have been a cleaner solution, but patchelf does not remove any dynamic section entries yet and I don't think supporting binaries that were broken in the fist place worth the burden of starting to do so.

And @Mic92, thank you for pointing this case out.

@Mic92
Copy link
Member

Mic92 commented Aug 10, 2021

I also gained now merge rights for this repo.

`patchelf --set-rpath` corrupted executables on mips32el: the dynamic
liker crushed with Segmentation fault when loading any executable with
RPATH added that way.

The problem was around the MIPS-specific mechanism of setting up the
debug map pointer. When DT_MIPS_RLD_MAP_REL entry in the dynamic section
is present, it holds the relative address of __RLD_MAP -- an offset
relative to this dynamic section entry. Dynamic linker puts the
pointer to the `r_debug` structure there.

When patchelf updates the executable RPATH, it moves the .dynamic
section both in the binary and in memory, while __RLD_MAP is not moved
in memory, since it belongs to special .rld_map section that has type
PROGBITS. So, the offset stored in DT_MIPS_RLD_MAP_REL entry is not
valid anymore and should be updated.

This commit adds the necessary update.

In the corner case when DT_MIPS_RLD_MAP_REL is present, but
.rld_map section is not, the dynamic loader writes the debug
pointer to some arbitrary bytes in memory. To avoid crushes
on otherwise "working" binaries, we set offset to zero
so that the dynamic loader would just overwrite the dynamic
section.

Here we also import DT_MIPS_RLD_MAP_REL definition in elf.h form
glibc commit a2057c984e4314c3740f04cf54e36c824e4c8f32.

Refs: NixOS#82
Signed-off-by: Ivan A. Melnikov <iv@altlinux.org>
When loading the executable on MIPS, the dynamic loader looks for MIPS
ABI flags using PT_MIPS_ABIFLAGS header. The flags themselves are stored
in the .MIPS.abiflags section, so the header must be updated when the
section is moved.

Here we also import PT_MIPS_ABIFLAGS definition from glibc commit
0bd956720c457ff054325b48f26ac7c91cb060e8.

Closes: NixOS#82
Signed-off-by: Ivan A. Melnikov <iv@altlinux.org>
@Mic92 Mic92 merged commit 0121f5e into NixOS:master Aug 11, 2021
@Mic92
Copy link
Member

Mic92 commented Aug 11, 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

5 participants