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

Avoid executable path resolution in buildkit-qemu-* #233

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

p1-0tr
Copy link
Collaborator

@p1-0tr p1-0tr commented Feb 3, 2025

The buildkit inline emulator resolves paths of the binary it executes.
This is needed for readlink()/open()/... to work on /proc/self/exe
correctly, in case the binary changes its working directory. However the
resolution may result in unexpected args values. So avoid passing the
resolved paths around, but use them when the emulated process accesses
/proc/self/exe.

Known issue - a shell script like #!./chwd will still not work as expected,
as it will set /proc/self/exe to the script, rather than the actual
executable.

The buildkit inline emulator resolves paths of the binary it executes.
This is needed for readlink()/open()/... to work on /proc/self/exe
correctly, in case the binary changes its working directory. However the
resolution may result in unexpected args values. So avoid passing the
resolved paths around, but use them when the emulated process accesses
/proc/self/exe.

Known issue - a shell script like `#!./chwd` will still not work as expected,
as it will set /proc/self/exe to the script, rather than the actual
executable.

Signed-off-by: Piotr Stankiewicz <[email protected]>
@p1-0tr p1-0tr force-pushed the ps-fix-exe-resolution branch from dca9ebe to b727548 Compare February 4, 2025 13:12
@p1-0tr p1-0tr changed the title WiP: Avoid executable path resolution Avoid executable path resolution in buildkit-qemu-* Feb 4, 2025
@p1-0tr p1-0tr marked this pull request as ready for review February 4, 2025 13:13
Copy link
Collaborator

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks

PTAL @tonistiigi

@crazy-max crazy-max requested a review from tonistiigi February 4, 2025 14:32
@tonistiigi tonistiigi merged commit 90e7f2a into tonistiigi:master Feb 5, 2025
46 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.

3 participants