amluto / virtme

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

mkinitramfs.py: Search for busybox{.,-}static first #52

Closed marcosps closed 4 years ago

marcosps commented 4 years ago

In currentl Tumbleweed, using busybox dynamic linked does not work. For now let's check if we have busybox static first.

Fix: #51

Signed-off-by: Marcos Paulo de Souza mpdesouza@suse.com

zevweiss commented 4 years ago

Funny, I was just looking at that code a few hours ago (unrelatedly to the ongoing discussion in #51) and thinking I probably should have put the static suffixes first in that list...

marcosps commented 4 years ago

@zevweiss so maybe this is the right/safe choice?

zevweiss commented 4 years ago

I'd think so, yeah -- busybox on its own is potentially kind of ambiguous (could be statically or dynamically linked), but it's presumably safe to assume that busybox[.-]static is, in fact, a static executable, so unless #13 (or something similar) is applied, trying the definitely-static ones first certainly seems appropriate.

Granted, I suppose even with this patch it'd still be vulernable to misbehaving if there's, say, a dynamic /usr/local/bin/busybox and a static /usr/bin/busybox.static for whatever reason; to be really thorough about it we could maybe do something like

for p in itertools.product(['-static', '.static', ''],
                           ['usr/local', 'usr', ''],
                           ['bin', 'sbin']):
    path = os.path.join(root, p[1], p[2], 'busybox' + p[0])

to search for an explicitly-static one everywhere before falling back to a maybe-dynamic one.

amluto commented 4 years ago

Yeah, let's do @zevweiss's version. Bonus points for parsing just enough of the ELF header to confirm it's static in lieu of actually doing #13. Although I'm not entirely opposed to having a nice mechanism to pull in dynamic dependencies, but it's certainly simpler and faster to use the static binary if we can find it.

okurz commented 4 years ago

I can confirm this patch fixes the problem I observed running virtme on openSUSE Leap 15.1 with virtme-run --installed-kernel --memory 2G :)

zevweiss commented 4 years ago

Hmm, though on a related topic I notice the README says, regarding the busybox binary, "somewhere in your path"; to actually match that, perhaps ideally we'd incorporate os.getenv('PATH').split(':') into the mix too...

amluto commented 4 years ago

I'm going to merge this PR and then improve it a bit on top.