[1.1] Fix failed exec after systemctl daemon-reload (regression in 1.1.3) #3554
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kolyshkin
force-pushed
the
1.1-fix-dev-pts
branch
from
ff7cdd1
to
98adcb5
Compare
kolyshkin
changed the title
|
CI failure is to be fixed by #3558 Still no regression test :( |
|
Tried a build from this PR, and that fixes the issue reported in moby/moby#43960 (reply in thread) |
kolyshkin
force-pushed
the
1.1-fix-dev-pts
branch
from
98adcb5
to
e6de45d
Compare
|
Thanks, but I don't see the main PR, isn't the main branch affected? |
This comment was marked as outdated.
This comment was marked as outdated.
kolyshkin
changed the title
thaJeztah
previously approved these changes
Aug 17, 2022
good point; PR for main is in #3559, but indeed still in draft; do we want the one in main first? |
This comment was marked as outdated.
This comment was marked as outdated.
kolyshkin
added
area/cgroupv2
area/systemd
area/ebpf
backport/1.1-pr
kolyshkin
force-pushed
the
1.1-fix-dev-pts
branch
from
e6de45d
to
7054801
Compare
AkihiroSuda
approved these changes
Aug 17, 2022
This comment was marked as outdated.
This comment was marked as outdated.
kolyshkin
force-pushed
the
1.1-fix-dev-pts
branch
from
7054801
to
8630d3f
Compare
AkihiroSuda
previously approved these changes
Aug 18, 2022
|
Please run git cherry-pick with -x for tracking the main commit |
That is what I usually do, although not in this case. The reasons are:
I can certainly add a reference to commit 58b1374 to the commit message. |
A regression reported for runc v1.1.3 says that "runc exec -t" fails after doing "systemctl daemon-reload": > exec failed: unable to start container process: open /dev/pts/0: operation not permitted: unknown Apparently, with commit 7219387 we are no longer adding "DeviceAllow=char-pts rwm" rule (because os.Stat("char-pts") returns ENOENT). The bug can only be seen after "systemctl daemon-reload" because runc also applies the same rules manually (by writing to devices.allow for cgroup v1), and apparently reloading systemd leads to re-applying the rules that systemd has (thus removing the char-pts access). The fix is to do os.Stat only for "/dev" paths. Also, emit a warning that the path was skipped. Since the original idea was to emit less warnings, demote the level to debug. Note this also fixes the issue of not adding "m" permission for block-* and char-* devices. A test case is added, which reliably fails before the fix on both cgroup v1 and v2. This is a backport of commit 58b1374 to release-1.1 branch. Fixes: opencontainers#3551 Fixes: 7219387 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
1.1-fix-dev-pts
branch
from
8630d3f
to
204c673
Compare
Done; PTAL @AkihiroSuda |
AkihiroSuda
approved these changes
Aug 18, 2022
|
@kolyshkin @AkihiroSuda @cyphar should we do a |
2 tasks
This was referenced Nov 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A regression reported for runc v1.1.3 says that after systemctl
daemon-reload runc exec fails:
Apparently, with commit 7219387 we are no longer adding
"DeviceAllow=char-pts rwm" rule (because os.Stat("char-pts") returns
ENOENT).
The bug can only be seen after "systemctl daemon-reload" because runc
also applies the same rules manually (by writing to devices.allow for
cgroup v1), and apparently reloading systemd leads to re-applying the
rules that systemd has (thus removing the char-pts access).
The fix is to do os.Stat only for "/dev" paths.
Also, emit a warning that the path was skipped. Since the original idea
was to emit less warnings, demote the level to debug.
Note this also fixes the issue of not adding "m" permission for block-*
and char-* devices.
A test case is added, which reliably fails before the fix
on both cgroup v1 and v2.
Fixes: #3551
Fixes: 7219387