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

Use hash table to determine number of dynamic symbols #109

Merged
merged 1 commit into from
Oct 14, 2018

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Oct 10, 2018

Partial fix for #106

Copy link
Owner

@m4b m4b left a comment

Choose a reason for hiding this comment

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

This is great work! I think you fixed #106, #109, and m4b/bingrep#19 !!!

} else if let Some(hash) = dyn_info.hash {
hash_len(bytes, hash as usize, header.e_machine, ctx)?
} else {
0
Copy link
Owner

Choose a reason for hiding this comment

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

Ok, was going to ask; in case of a non-existent gnu/hash table, we effectively don't parse any symbols. I'm ok with this, but now I'm wondering (from a purely scientific curiosity):

  1. Can dynamic libraries not have a hash table
  2. How does the dynamic linker compute dynsym size if no? I imagine they must have hash tables if they're dynamic libraries?

Choose a reason for hiding this comment

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

  1. No, dynamic libraries must have a hash table (SYSV or GNU_HASH). Nevertheless the table can be empty (c.f. https://github.com/lief-project/LIEF/blob/master/src/ELF/Builder.cpp#L95-L133)
    Object file (.o) don't have hash table, neither dynamic symbols.
  2. Actually it doesn't compute the number of symbols. It look for an index and check that this index is consistent

Copy link
Owner

Choose a reason for hiding this comment

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

@romainthomas awesome, thanks for the clarification! It's super annoying to me the _DYNAMIC doesn't provide a DT_SYMTABSZ, like it does for basically everything else:

   DT_INIT_ARRAY 0x3d9060
 DT_INIT_ARRAYSZ 0x8
   DT_FINI_ARRAY 0x3d9068
 DT_FINI_ARRAYSZ 0x8
     DT_GNU_HASH 0x413548
       DT_STRTAB 0x415210
       DT_SYMTAB 0xc90
        DT_STRSZ 0x2554
       DT_SYMENT 0x18

Choose a reason for hiding this comment

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

Yes I think they forgot this very important information in the ELF spec. Nice project btw ;)

Copy link
Owner

Choose a reason for hiding this comment

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

thanks :) if you ever felt like swapping out a backend, I'd definitely be interested in spec-ing out/ designing some C bindings for goblin (though it will probably be a lot of work?)

@m4b m4b merged commit b4f82a6 into m4b:master Oct 14, 2018
fn gnu_hash_len(bytes: &[u8], offset: usize, ctx: Ctx) -> error::Result<usize> {
let buckets_num = bytes.pread_with::<u32>(offset, ctx.le)? as usize;
let min_chain = bytes.pread_with::<u32>(offset + 4, ctx.le)? as usize;
let bloom_size = bytes.pread_with::<u32>(offset + 8, ctx.le)? as usize;
Copy link
Owner

Choose a reason for hiding this comment

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

looking again, gread_with would have been better here to automate the offset computations, but will update another time :)

@philipc philipc deleted the dyn-hash branch October 14, 2018 21:21
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