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

Add feature to rename dynamic symbols #459

Merged
merged 14 commits into from
Feb 23, 2023
Merged

Conversation

brenoguim
Copy link
Collaborator

@brenoguim brenoguim commented Feb 18, 2023

Hello!

This PR enhances Patchelf to be able to change the name of dynamic symbols in shared libraries and executables.

This feature is similar to what is available in https://lief-project.github.io//doc/latest/tutorials/03_elf_change_symbols.html.

This can be useful when you need to change symbols, but don't have access to the source code (or object files).

The patch grew quite long, so these are the keypoints:
It adds a swich that takes a file as argument. This file is expected to have a sequence of pairs of symbol names. So if the file contains "A B C D", the symbol "A" will be renamed to "B" and symbol "C" will be renamed to "D".

First step is to iterate on the symbol table, checking their names. If it's a symbol to be renamed, append the new name to the string table, and update the symbol entry to point to the new name.
This is quite wasteful, but it should be possible to do better in the future.

Next step is to update the hash sections. Happily, they don't change in size, just content, so we just have to update it.

  • .hash is quite straightforward and I recommend start by reviewing that one.
  • .gnu_hash is similar to .hash but adds two extra complications: it has a bloom filter and it imposes that symbols in the same buckets must be adjacent in the array of symbols.

Unfortunately, changing the order of entries in the symbol array forces us to fix three other sections:
.gnu.version which must be in sync with the symbol table. To solve this, I just create a map of "old" position to "new" position and reorder based on that.
.rela.dyn and .rela.plt - the location tables store indices referring to the symbol table. The old2new map is also used here.

I also tried to improve a bit of the ergonomics by adding a very lightweight span class to represent views over arrays (https://en.cppreference.com/w/cpp/container/span will come out in c++20). It drastically the amount of pairs "pointer and size" in the code.
Also added getSectionSpan and tryGetSectionSpan to go with them.

Testing done:
Added a test that checks the symbol table, hash table (only gnu), the version table, and the relocation tables.
I also tested it on a large software with many libraries. I renamed around 600k symbols over hundreds of shared libraries, and the software runs. That's actually how I found the missing sections to update.

@brenoguim brenoguim force-pushed the breno.rename_syms branch 16 times, most recently from e998f55 to 18a8b8a Compare February 18, 2023 19:39
@brenoguim brenoguim changed the title WIP: Add feature to rename dynamic symbols Add feature to rename dynamic symbols Feb 18, 2023
@brenoguim
Copy link
Collaborator Author

brenoguim commented Feb 19, 2023

I think the CI failed because my test is trying to patch the libstdc++ of the system by mistake because it was a link. Just a guess. I'll get to it.

Thanks for runnings the CI :)

@brenoguim
Copy link
Collaborator Author

The failure doesn't tell me much. Just that patchelf could not write the libstdc++.so file. I'm a bit in the dark, but that could happen if the file cannot be written, so I added another change to try to make it work.

@Mic92
Copy link
Member

Mic92 commented Feb 20, 2023

@brenoguim you can add https://github.com/marketplace/actions/debugging-with-tmate to your own github fork in order to debug this (and in case you cannot reproduce this locally)

@brenoguim brenoguim force-pushed the breno.rename_syms branch 2 times, most recently from e6f75d5 to 62dcdd2 Compare February 20, 2023 17:17
@brenoguim
Copy link
Collaborator Author

Thanks @Mic92 . For some reason I thought this CI required private servers, so I didn't even realize I could run it on my fork.

I'll have to rework the tests a little bit because I was renaming all symbols from lib*c++.so and making sure patchelf still runs fine.
However there are some tests linking everything statically, so that wouldn't work. I'll create a binary+library for the test.
It makes it less extensive but maybe it could be enhanced in the future (for example to include checks for version symbols).

@brenoguim
Copy link
Collaborator Author

Hey @Mic92 , the CI is clean on my fork. It should work now.

src/patchelf.cc Outdated Show resolved Hide resolved
src/patchelf.h Show resolved Hide resolved
src/patchelf.h Outdated Show resolved Hide resolved
src/patchelf.cc Outdated Show resolved Hide resolved
@Mic92
Copy link
Member

Mic92 commented Feb 21, 2023

Sorry for these incremental reviews, but it always takes me a bit of context switch to get back into this project...

src/patchelf.cc Show resolved Hide resolved
src/patchelf.cc Show resolved Hide resolved
src/patchelf.cc Outdated Show resolved Hide resolved
patchelf.1 Show resolved Hide resolved
@Mic92
Copy link
Member

Mic92 commented Feb 22, 2023

I did now a bit more manual testing an found the following rough edges above.

brenoguim and others added 6 commits February 22, 2023 06:10
Co-authored-by: Jörg Thalheim <Mic92@users.noreply.github.com>
Co-authored-by: Jörg Thalheim <Mic92@users.noreply.github.com>
Co-authored-by: Jörg Thalheim <Mic92@users.noreply.github.com>
Untested. Coding from github while my VM is acting up.
@brenoguim
Copy link
Collaborator Author

Sorry for changing the code in the middle of review. I noticed I was using the syntax for template lambdas auto x = [] <class ER> () { ... } and that is a C++20 thing that compilers accept in C++17 mode. Still, we want to keep it C++17.
So the last patch does that.
I also moved the helper functions to the private section.

@Mic92
Copy link
Member

Mic92 commented Feb 23, 2023

bors merge

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

3 participants