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

elf: Detect if Elf is PIE #5126

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Conversation

Rtoax
Copy link
Contributor

@Rtoax Rtoax commented Oct 27, 2024

It is not sufficient to determine whether Elf is ET_DYN or not to be a dynamic library in bcc_elf_is_shared_obj() function, because the executable Elf of the PIE type is also ET_DYN.

For example, on fedora 40, /usr/bin/bash is PIE:

    $ readelf -h /usr/bin/bash
    ELF Header:
      ...
      Type:                     DYN (Position-Independent Executable file)
      ...

bcc_elf_is_shared_obj() should not return 'true' for /usr/bin/bash.

This commit add a function bcc_elf_is_pie(), maybe we can provide API in bcc_elf.h.

@yonghong-song
Copy link
Collaborator

There is a test failure:

raceback (most recent call last):
  File "/bcc/tests/python/test_usdt3.py", line 146, in test_attach1
    self.assertTrue(self.probe_value_3 != 0)
AssertionError: False is not true

could you take a look?

Searching internet, I found the following link:
https://stackoverflow.com/questions/34519521/why-does-gcc-create-a-shared-object-instead-of-an-executable-binary-according-to/55704865#55704865
which seems validating your implementation.
One thing the above link mentioned is for a shared library, if that shared library is executable, it will be clarified as an executable, e.g. ld.so. But ld.so is a special case, so I guess your implementation should be okay. Not sure why test_usdt3.py failed and whether it is related to your patch or not. It will be great if you can double check.

@Rtoax
Copy link
Contributor Author

Rtoax commented Oct 30, 2024

There is a test failure:

raceback (most recent call last):
  File "/bcc/tests/python/test_usdt3.py", line 146, in test_attach1
    self.assertTrue(self.probe_value_3 != 0)
AssertionError: False is not true

could you take a look?

Searching internet, I found the following link: https://stackoverflow.com/questions/34519521/why-does-gcc-create-a-shared-object-instead-of-an-executable-binary-according-to/55704865#55704865 which seems validating your implementation. One thing the above link mentioned is for a shared library, if that shared library is executable, it will be clarified as an executable, e.g. ld.so. But ld.so is a special case, so I guess your implementation should be okay. Not sure why test_usdt3.py failed and whether it is related to your patch or not. It will be great if you can double check.

Thanks for your reply, Song. I think i just found the ISSUE:

In src/cc/usdt/usdt_args.cc::get_global_address() call bcc_elf_is_shared_obj().

bcc uses Ubuntu 20.04 as the CI/CD environment on github. The test ELF program will be compiled as PIE. After that, bcc_elf_is_shared_obj() will skip PIE, causing the py_test_usdt3 test to fail.

Similarly, it will be compiled as PIE on Debian-like systems and as EXEC on Fedora-like systems.

On Fedora 40

      Start 30: py_test_usdt3
30/37 Test #30: py_test_usdt3 ....................   Passed    2.30 sec

On Debian12

    Start 2: py_test_usdt3
30/37 Test #2: py_test_usdt3 ....................***Failed   67.42 sec

It is not sufficient to determine whether Elf is ET_DYN or not to be a
dynamic library in bcc_elf_is_shared_obj() function, because the executable
Elf of the PIE type is also ET_DYN.

For example, on Fedora 41, /usr/bin/bash is PIE:

    $ readelf -h /usr/bin/bash
    ELF Header:
      ...
      Type:                     DYN (Position-Independent Executable file)
      ...

bcc_elf_is_shared_obj() should not return 'true' for /usr/bin/bash.

This commit add function bcc_elf_is_pie() and export it in bcc_elf.h.

At the same time, fix test py_test_usdt3.

Signed-off-by: Rong Tao <[email protected]>
Signed-off-by: Jiang Guirong <[email protected]>
@Rtoax
Copy link
Contributor Author

Rtoax commented Nov 3, 2024

Rebase to master and squash commits to single one.

@yonghong-song yonghong-song merged commit 0b5be9b into iovisor:master Nov 4, 2024
12 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