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

Check for SHN_UNDEF during symbol dumping #574

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Conversation

sevaa
Copy link
Contributor

@sevaa sevaa commented Oct 23, 2024

Supersedes #566

Now, I don't know what was the original motivation for that one, but this fix should be equivalent.

@eliben
Copy link
Owner

eliben commented Oct 23, 2024

I like the change more but agree that the motivation remains mysterious, especially without any test case. Is this similar to what readelf itself is doing in a comparable code path?

@sevaa
Copy link
Contributor Author

sevaa commented Oct 26, 2024

There is a similar check in readelf when it retrieves the symbol name for STT_SECTION type symbols. In readelf's print_symbol() there is the following:

/* Get the symbol's name.  For section symbols without a
     specific name use the (already computed) section name.  */
  if (ELF_ST_TYPE (psym->st_info) == STT_SECTION
      && section_index_real (filedata, psym->st_shndx)
      && psym->st_name == 0)
    {
      ;
    }
  else
    {
      bool is_valid;

      is_valid = valid_symbol_name (strtab, strtab_size, psym->st_name);
      sstr = is_valid  ? strtab + psym->st_name : _("<corrupt>");
    }

Where section_index_real() checks, among other things, if st_shndx>0. So as far as understand, the logic is:

  • if st_shndx is nonzero, treat it as a section index, retrieve that section's name
  • if not, retrieve the name from the string table at st_name
  • if that is not available either, use <corrupt>

So this tells me that st_shndx being zero (SHN_UNDEF) is a condition readelf handles gracefully. So pyelftools shouldn't crash on that either :)

@eliben eliben merged commit d0b50a2 into eliben:main Oct 28, 2024
4 checks passed
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.

2 participants