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

cmd: fix re-exec bug with classic confinement for host snapd < 2.28 #4176

Merged
merged 2 commits into from
Nov 9, 2017

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Nov 8, 2017

Older version of snapd (before 2.28) did use the SNAPD_DID_REEXEC
env to check if they should re-exec or not. We need
to unset it after re-exec because the host snap tool may be old and
using this key.

So if e.g. the host has snapd 2.27 and snapd re-execs to 2.29 then
snap run --shell classic-snap will go into an environment where the
snapd 2.27 sees this key and stops re-execing and things break in
suble ways.

C.f.
https://forum.snapcraft.io/t/seccomp-error-calling-snap-from-another-classic-snap-on-core-candidate/2736/7

@mvo5 mvo5 added this to the 2.29 milestone Nov 8, 2017
@mvo5 mvo5 added the ⚠ Critical High-priority stuff (e.g. to fix master) label Nov 8, 2017
Older version of snapd (before 2.28) did use the SNAPD_DID_REEXEC
env to check if they should re-exec or not. We need
to unset it after re-exec because the host snap tool may be old and
using this key.

So if e.g. the host has snapd 2.27 and snapd re-execs to 2.29 then
`snap run --shell classic-snap` will go into an environment where the
snapd 2.27 sees this key and stops re-execing and things break in
suble ways.

C.f.
https://forum.snapcraft.io/t/seccomp-error-calling-snap-from-another-classic-snap-on-core-candidate/2736/7
Copy link
Contributor

@chipaca chipaca left a comment

Choose a reason for hiding this comment

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

🤦

Copy link
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

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

Re-exec deserves a post-mortem analysis.

+1

@codecov-io
Copy link

codecov-io commented Nov 9, 2017

Codecov Report

Merging #4176 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4176      +/-   ##
==========================================
+ Coverage   75.53%   75.56%   +0.02%     
==========================================
  Files         436      436              
  Lines       37811    37822      +11     
==========================================
+ Hits        28562    28581      +19     
+ Misses       7253     7245       -8     
  Partials     1996     1996
Impacted Files Coverage Δ
cmd/cmd.go 89.21% <100%> (+5.34%) ⬆️
overlord/ifacestate/helpers.go 60.26% <0%> (+0.66%) ⬆️
interfaces/kmod/kmod.go 92.3% <0%> (+19.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1b51b5...4cce6e8. Read the comment docs.

@mvo5 mvo5 merged commit 10fcc00 into canonical:master Nov 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠ Critical High-priority stuff (e.g. to fix master)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants