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

Reexec with /proc/self/exe on Linux #11283

Merged
merged 4 commits into from
Mar 25, 2022
Merged

Conversation

espadolini
Copy link
Contributor

@espadolini espadolini commented Mar 21, 2022

Right now we're using the output of os.Executable to decide which binary to launch for our internal teleport exec/teleport forward/teleport checkhomedir reexecs. If the binary is replaced, for instance during an upgrade, we end up calling the new binary rather than the old one - which works fine but relies on the new version working correctly and having the exact semantics. If the binary is deleted (by accident, for instance) the tsh ssh functionality breaks, which could be problematic if Teleport is the only way to remote into a system.

We don't have a better way on Darwin, but on Linux (starting from Linux 2.2) running execve directly on /proc/self/exe will execute the currently running binary no matter what happened to any paths referring to it. To compensate for a qemu-user bug (see https://gitlab.com/qemu-project/qemu/-/issues/927) and general qemu-user weirdness, we do a runtime check at init time to try to detect if we're in a qemu-user that is misbehaving, and if not, we end up opening our binary and then reexecuting it via /proc/self/fd/N.

Closes #11052.

@espadolini espadolini force-pushed the espadolini/reexec-procfs branch 3 times, most recently from 5b6cc3c to 623b20e Compare March 22, 2022 17:16
@espadolini espadolini marked this pull request as ready for review March 22, 2022 17:18
@github-actions github-actions bot requested review from lxea and nklaassen March 22, 2022 17:37
Copy link
Contributor

@nklaassen nklaassen left a comment

Choose a reason for hiding this comment

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

This won't affect graceful restart, right? Since that is often used for upgrades and we'd want to load the new binary

lib/srv/reexec_linux.go Outdated Show resolved Hide resolved
@espadolini
Copy link
Contributor Author

Yes, (*TeleportProcess).forkChild() (which is what is called on a HUP or USR2) uses exec.LookPath(os.Args[0]) as the executable unconditionally, whereas the reexec machinery (used to launch user shells) uses os.Executable() and is affected by this change.

@espadolini espadolini force-pushed the espadolini/reexec-procfs branch 2 times, most recently from ff6daf7 to 51a3765 Compare March 24, 2022 16:12
nklaassen
nklaassen previously approved these changes Mar 24, 2022
@espadolini espadolini force-pushed the espadolini/reexec-procfs branch from 51a3765 to b02107d Compare March 24, 2022 17:56
@espadolini espadolini requested a review from nklaassen March 24, 2022 17:57
@espadolini espadolini dismissed nklaassen’s stale review March 24, 2022 17:57

changed the qemu-user workaround

@espadolini espadolini force-pushed the espadolini/reexec-procfs branch 3 times, most recently from 8b7f9a4 to bf35c15 Compare March 24, 2022 19:19
@espadolini espadolini force-pushed the espadolini/reexec-procfs branch from bf35c15 to d68ff0b Compare March 24, 2022 19:20
@espadolini
Copy link
Contributor Author

@nklaassen @lxea PTAL, this should be the final version

lib/srv/reexec_linux.go Show resolved Hide resolved
Copy link
Contributor

@lxea lxea left a comment

Choose a reason for hiding this comment

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

Thanks for the in-depth explanations also!

@espadolini espadolini enabled auto-merge (squash) March 25, 2022 09:52
@espadolini espadolini merged commit 4384c35 into master Mar 25, 2022
@espadolini espadolini deleted the espadolini/reexec-procfs branch March 25, 2022 10:16
espadolini added a commit that referenced this pull request Mar 25, 2022
* Reexec with `/proc/self/exe` on Linux

* Add a check for qemu-user

* Add comment
espadolini added a commit that referenced this pull request Mar 25, 2022
* Reexec with `/proc/self/exe` on Linux

* Add a check for qemu-user

* Add comment
espadolini added a commit that referenced this pull request Mar 25, 2022
* Reexec with `/proc/self/exe` on Linux

* Add a check for qemu-user

* Add comment
@espadolini espadolini added the robustness Resistance to crashes and reliability label Mar 25, 2022
espadolini added a commit that referenced this pull request Mar 25, 2022
* Reexec with `/proc/self/exe` on Linux

* Add a check for qemu-user

* Add comment
espadolini added a commit that referenced this pull request Mar 25, 2022
* Reexec with `/proc/self/exe` on Linux

* Add a check for qemu-user

* Add comment
espadolini added a commit that referenced this pull request Mar 25, 2022
* Reexec with `/proc/self/exe` on Linux

* Add a check for qemu-user

* Add comment
@webvictim webvictim mentioned this pull request Apr 19, 2022
@webvictim webvictim mentioned this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
robustness Resistance to crashes and reliability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SIGHUP multiple times results in supervising process N which is not our child errors and no connectivity.
3 participants