Skip to content

Conversation

@marcosps
Copy link
Contributor

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

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@zevweiss
Copy link
Contributor

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
Copy link
Owner

amluto commented Oct 17, 2019

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
Copy link

okurz commented Oct 17, 2019

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
Copy link
Contributor

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
Copy link
Owner

amluto commented Oct 17, 2019

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

@amluto amluto merged commit 92c7558 into amluto:master Oct 17, 2019
@marcosps marcosps deleted the mpdesouza_busybox_tumbleweed branch October 18, 2019 02:30
amluto added a commit that referenced this pull request Oct 18, 2019
Now the algorithm prefers the 'static' versions regardless of path,
and it searches PATH first (for native) as advertised.

See issue #52.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
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.

virtme-run --installed-kernel is currently broken in the mos recent tumbleweed snapshot

4 participants