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

Fix ELF R_X86_64_PLT32 relocation entries patching ##bin #17587

Merged
merged 4 commits into from Sep 8, 2020

Conversation

ret2libc
Copy link
Contributor

@ret2libc ret2libc commented Sep 4, 2020

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the documentation and the radare2 book with the relevant information (if needed)

Detailed description

See issue, it's well described.

Test plan

See radareorg/radare2-testbins#49.

Closing issues

Closes #17583

@ret2libc ret2libc marked this pull request as draft September 4, 2020 08:29
@ret2libc ret2libc removed the request for review from trufae September 4, 2020 08:29
Correctly get the address of the PLT entry for a symbol. If the symbol
was not yet added to the .got.r2 section, then take the first available
address (vaddr), otherwise retrieve it from the hashtable.
@ret2libc ret2libc marked this pull request as ready for review September 4, 2020 08:41
@ret2libc
Copy link
Contributor Author

ret2libc commented Sep 4, 2020

Ok apparently it is not working well yet. Moving it back to draft :(

@ret2libc ret2libc marked this pull request as draft September 4, 2020 08:57
@github-actions github-actions bot added the RBin label Sep 4, 2020
@XVilka
Copy link
Contributor

XVilka commented Sep 4, 2020

See also #17300 and #17523.

@ret2libc
Copy link
Contributor Author

ret2libc commented Sep 4, 2020

See also #17300 and #17523.

Very different problem.

@@ -1512,13 +1512,6 @@ static int handleMidFlags(RCore *core, RDisasmState *ds, bool print) {
} else if (!strncmp (fi->name, "str.", 4)) {
ds->midflags = R_MIDFLAGS_REALIGN;
} else if (!strncmp (fi->name, "reloc.", 6)) {
if (print) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not only it seems unnecessary because, as written in the comment, there is already a comment for relocations. But it is also wrong in some cases.

RBinReloc *rel;
rel = r_core_getreloc (ds->core, ds->analop.addr, ds->analop.size);
RBinReloc *rel = NULL;
if (!ds->core->bin->is_reloc_patched) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GustavoLCR I saw this was done by you in #14587 to correctly show relocations when -e io.cache=true is not used. However, when that's used this causes issues because the relocations AFAIK change positions.

@ret2libc ret2libc marked this pull request as ready for review September 4, 2020 12:56
@XVilka XVilka added this to the 4.6.0 milestone Sep 7, 2020
@XVilka XVilka requested a review from trufae September 8, 2020 02:30
@ret2libc ret2libc merged commit 272265a into radareorg:master Sep 8, 2020
@trufae trufae mentioned this pull request Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

R_X86_64_PLT32 relocation types in relocatable objects
2 participants