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 dt needed inside dt dynamic struct #17009

Merged
merged 15 commits into from Jun 11, 2020
Merged

Add dt needed inside dt dynamic struct #17009

merged 15 commits into from Jun 11, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 5, 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
Add the dt_needed array.
So i could remove the old dyn entries buffer.

This PR need the validation of the pr #17004

Test plan
One regression test fail. But for me the test is false.

The expected value for the test suite

-vaddr      paddr      type   name
----------------------------------
-0x00600988 0x00000988 SET_64 __gmon_start__
-0x006009a8 0x000009a8 SET_64 write
-0x006009b0 0x000009b0 SET_64 close
-0x006009b8 0x000009b8 SET_64 __libc_start_main
-0x006009c0 0x000009c0 SET_64 open
❯ readelf --relocs --use-dynamic ./test/bins/elf/analysis/dynamic-poffset

'RELA' relocation section at offset 0x0 contains 17179869188 bytes:
readelf: Warning: Virtual address 0x0 not located in any PT_LOAD segment.
readelf: Error: Reading 17179869188 bytes extends past end of file for 64-bit relocation data

The dynamic section is invalid.

Closing issues
Work on #12732

@ghost ghost requested review from kazarmy and ret2libc as code owners June 5, 2020 16:24
@lgtm-com
Copy link

lgtm-com bot commented Jun 5, 2020

This pull request introduces 1 alert when merging 1614af3 into d236343 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@ret2libc ret2libc self-assigned this Jun 8, 2020
@ghost
Copy link
Author

ghost commented Jun 8, 2020

@ret2libc Okay i just found one little problem.
the definition of Elf_Dyn is

           typedef struct {
               Elf32_Sword    d_tag;
               union {
                   Elf32_Word d_val;
                   Elf32_Addr d_ptr;
               } d_un;
           } Elf32_Dyn;
           extern Elf32_Dyn _DYNAMIC[];

           typedef struct {
               Elf64_Sxword    d_tag;
               union {
                   Elf64_Xword d_val;
                   Elf64_Addr  d_ptr;
               } d_un;
           } Elf64_Dyn;
           extern Elf64_Dyn _DYNAMIC[];

But the dyn_info only use Elf_(Xword). Or Elf_(Xword) != Elf_(Word).
Maybe in an other pr i will create a new Macro Elf_(Val).
It is why for DT_NEEDED entry i used Elf_(Off)

@ret2libc
Copy link
Contributor

ret2libc commented Jun 9, 2020

@ret2libc Okay i just found one little problem.
the definition of Elf_Dyn is

           typedef struct {
               Elf32_Sword    d_tag;
               union {
                   Elf32_Word d_val;
                   Elf32_Addr d_ptr;
               } d_un;
           } Elf32_Dyn;
           extern Elf32_Dyn _DYNAMIC[];

           typedef struct {
               Elf64_Sxword    d_tag;
               union {
                   Elf64_Xword d_val;
                   Elf64_Addr  d_ptr;
               } d_un;
           } Elf64_Dyn;
           extern Elf64_Dyn _DYNAMIC[];

But the dyn_info only use Elf_(Xword). Or Elf_(Xword) != Elf_(Word).
Maybe in an other pr i will create a new Macro Elf_(Val).
It is why for DT_NEEDED entry i used Elf_(Off)

If in doubt, you can as well just use ut64 and cast everything to ut64. It will work for both Elf32 and Elf64, I think.

@ret2libc
Copy link
Contributor

Ok, looks better thanks! Please update the tests as well and I think this looks good ;)

@ghost ghost requested a review from radare as a code owner June 10, 2020 17:06
@ret2libc ret2libc merged commit fa7b8ea into radareorg:master Jun 11, 2020
@ghost ghost deleted the add_dt_needed_inside_dt_dynamic_struct branch June 11, 2020 09:16
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.

1 participant