-
-
Notifications
You must be signed in to change notification settings - Fork 493
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
avoid string table duplicates and DT_NEEDED reference corruption when using --replace-needed multiple times #237
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d9c67d2
to
0856c88
Compare
Mic92
reviewed
Sep 20, 2020
0856c88
to
12d7e7f
Compare
12d7e7f
to
7e4532e
Compare
pranavkapoor001
pushed a commit
to ArrowOS-Devices/android_device_asus_X00T
that referenced
this pull request
Jan 16, 2021
* Turns out patchelf (from version 0.9 - 0.10) has trouble working with '--replace-needed' option (see NixOS/patchelf#237), as it corrupted the goodix lib we just patched from extract-utils. * Though I'm still not sure as to why it's seemingly working with few binaries and straight out corrupting others. * Since the host patchelf "seems" to be working just fine, revert back to using it. Signed-off-by: Saurabh Charde <[email protected]>
Swamoy14
pushed a commit
to Swamoy14/android_device_asus_X00T_4.4
that referenced
this pull request
Jan 18, 2021
* Turns out patchelf (from version 0.9 - 0.10) has trouble working with '--replace-needed' option (see NixOS/patchelf#237), as it corrupted the goodix lib we just patched from extract-utils. * Though I'm still not sure as to why it's seemingly working with few binaries and straight out corrupting others. * Since the host patchelf "seems" to be working just fine, revert back to using it. Signed-off-by: Saurabh Charde <[email protected]>
When it happens that the .gnu.version_r stores the strings in .dynstr it can come to corruption of the library names written into DT_NEEDED: -the library names in DT_NEEDED are replaced, new entries are written to the end of .dynstr -the version library names are replaced, and written to the end of the string section. If the section for the version strings is also ".dynstr", the previous modifications were _not_ taken into account and things were written from the old end of .dynstr again. The order in which these strings were written is not the same as the previous replacement, so things would end up with the same size, but different offsets. The .gnu.version_r table is correct, the file contents are fine, but the offsets in the DT_NEEDED entries are wrong. Since they are printed as 0-terminated strings the first one replaced will always be shown correct, which also is the case if the argument is only used once as the string is replaced with itself afterwards.
This can happen especially if .gnu.version_r stores the strings in .dynstr, so replacing the library names would add them twice to the same section. Keep a map of what was already added and where, and simply reuse the old entries if they are needed again.
Thanks! I rebased and merged this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When it happens that the .gnu.version_r stores the strings in .dynstr it can come to corruption of the library names written into DT_NEEDED (#158):
If the section for the version strings is also ".dynstr", the previous modifications were not taken into account and things were written from the old end of .dynstr again. The order in which these strings were written is not the same as the previous replacement, so things would end up with the same size, but different offsets. The .gnu.version_r table is correct, the file contents are fine, but the offsets in the DT_NEEDED entries are wrong. Since they are printed as 0-terminated strings the first one replaced will always be shown correct, which also is the case if the argument is only used once as the string is replaced with itself afterwards.
Now things are correct, but the strings are added to .dynstr twice. Use an unordered map to cache them.