amluto / virtme

An easy way to virtualize the running system
GNU General Public License v2.0
330 stars 66 forks source link

Copy DT_NEEDED files when installing busybox #77

Open jadahl opened 2 years ago

jadahl commented 2 years ago

From the second commit:

mkinitramfs: Copy DT_NEEDED files if any too

This fixes using busybox installations that aren't really static, e.g.
the one in Fedora 36 and 37, since they still need
/lib/ld-musl-x86_64.so.1 to run.

As also mentioned in the commit message, this makes use of pyelftools (pip install pyelftools) to do the actual ELF parsing.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2079295

jadahl commented 2 years ago

A different approach to a similar problem exist in https://github.com/amluto/virtme/pull/13

JonathonReinhart commented 2 years ago

In github.com/JonathonReinhart/staticx I faced a similar decision and went with parsing ldd output over parsing elf headers with pyelftools for the same reasons: automatic full flattened dependency tree and ld path preference handling.

On Wed, Aug 17, 2022, 19:57 Zev Weiss @.***> wrote:

@.**** commented on this pull request.

Neat...

As compared to #13 https://github.com/amluto/virtme/pull/13, this approach has the advantage of avoiding fork/exec and parsing command output, which is nice.

The ldd-based approach has the advantages of being a bit more thorough (it gets transitive DSO dependencies and /etc/ld.so.conf handling "for free", whereas I believe this patch would require additional work for both) and avoids adding a dependency on a third-party python module, which I think would be a first for this project.

Personally I appreciate the easier out-of-the-box usability of not having to mess with adding python modules, but would be basically OK with whichever @amluto https://github.com/amluto prefers.

In virtme/mkinitramfs.py https://github.com/amluto/virtme/pull/77#discussion_r948527345:

@@ -14,6 +14,32 @@ import itertools from . import cpiowriter from . import util +from elftools.elf.elffile import ELFFile +from elftools.elf.dynamic import DynamicSection + +ld_paths = ['/lib', '/lib65', '/usr/lib', '/usr/lib64']

lib65 here looks like a typo...

In virtme/mkinitramfs.py https://github.com/amluto/virtme/pull/77#discussion_r948527954:

@@ -14,6 +14,32 @@ import itertools from . import cpiowriter from . import util +from elftools.elf.elffile import ELFFile +from elftools.elf.dynamic import DynamicSection + +ld_paths = ['/lib', '/lib65', '/usr/lib', '/usr/lib64'] + +def is_dt_needed(tag):

  • return tag.entry.d_tag == 'DT_NEEDED'
  • +def find_library_path(needed):

  • for ld_path in ld_paths:
  • ld_path = f'{ld_path}/{needed}'

If for no other reason than consistency with the rest of the code it might be slightly preferable to use os.path.join() here.

— Reply to this email directly, view it on GitHub https://github.com/amluto/virtme/pull/77#pullrequestreview-1076531438, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOT5FVGLRRD6VJM2F6UT2LVZV37RANCNFSM563HKFZQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

jadahl commented 2 years ago

The ldd-based approach has the advantages of being a bit more thorough (it gets transitive DSO dependencies and /etc/ld.so.conf handling "for free", whereas I believe this patch would require additional work for both) and avoids adding a dependency on a third-party python module, which I think would be a first for this project.

Yea, I almost went and started to parse that file, as well as $LD_LIBRARY_PATH, but decided it was for another day :P

FWIW, I also then tested #13 and it appears to worked as well.

I thought about parsing ldd, but couldn't find any information about the output format being actually specified according to some "API". I realize it likely hasn't changed in decades, so perhaps over pedantic.