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

DT_NEEDED not listed for ELF32 files. #4

Closed
e5150 opened this issue Nov 13, 2016 · 5 comments
Closed

DT_NEEDED not listed for ELF32 files. #4

e5150 opened this issue Nov 13, 2016 · 5 comments

Comments

@e5150
Copy link

e5150 commented Nov 13, 2016

On line 504 getlibraries() in elfls.c:
count = proghdr[i].p_filesz / sizeof *dyns;
Where sizeof(*dyns) == sizeof(Elf64_Dyn). Given a 32-bit ELF file, where count is half of what it's supposed to be, given sufficiently many dynamic sections, DT_STRSZ or DT_STRTAB might be beyond the erronuous count, and getlibraries returns 0 prematurely, without listings any needed libraries.

e5150 added a commit to e5150/dst that referenced this issue Nov 13, 2016
@BR903
Copy link
Owner

BR903 commented Nov 14, 2016

Good catch. I've submitted a fix.

@e5150
Copy link
Author

e5150 commented Nov 14, 2016

The new version segfaults on me, due to the dyns = malloc on the new line 504 not allocating enough for 32-bit files, since p_filesz is count * sizeof(Elf32_Dyn), but dyns needs to hold count number of Elf64_Dyn.
Allocating p_filesz * 2 solves the issue, but is rather ugly…

@BR903
Copy link
Owner

BR903 commented Nov 14, 2016

The new version segfaults on me, due to the dyns = malloc on the
new line 504 not allocating enough for 32-bit files, since
p_filesz is count * sizeof(Elf32_Dyn), but dyns needs to hold
count number of Elf64_Dyn. Allocating p_filesz * 2 solves the
issue, but is rather ugly?

Ugh! I must have messed up my test somehow.

BR903 added a commit that referenced this issue Nov 14, 2016
This addresses issue #4, again. Hopefully for real this time.
@BR903
Copy link
Owner

BR903 commented Nov 14, 2016

I've submitted a replacement fix. Less elegant, but this one actually works.

@e5150
Copy link
Author

e5150 commented Nov 15, 2016

Yes, now I'm getting the expected results on the files that first brought the issue to light.
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

No branches or pull requests

2 participants