Fix failed exec after systemctl daemon-reload#3559
Fix failed exec after systemctl daemon-reload#3559kolyshkin merged 1 commit intoopencontainers:mainfrom
Conversation
|
In the commit message could you explain how to test this? |
|
Is this still draft? I think we want to merge this main PR first and then cherry-pick |
|
Looks like it was @kolyshkin's intention to merge the 1.1 fix first, then forward-port to main;
Have we done this before? I think generally we go from |
|
Got some progress with the test case in #3555. Will add the test case here once it's working. Draft for now. While originally this is a forward port (because the bug was reported against 1.1 and I initially fixed it in there), let's merge this one first, and then follow up with 1.1. |
kolyshkin
added
area/systemd
area/ebpf
backport/1.1-done
|
Draft pending test case addition. |
|
Ready for review. The |
|
The test failure before the fix can be seen in PR #3555 |
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
do we want to ignore the code spell issue, or should we do a rebase? (probably that requires re-cherry-picking for the release branch though)
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. Fixes: opencontainers#3551 Fixes: 7219387 Signed-off-by: Kir Kolyshkin <[email protected]>
4 participants
This is a forward port of #3554 to the main branch. Original description follows.
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 (for a test run before the fix, see #3555).
Fixes: #3551
Fixes: 7219387
Signed-off-by: Kir Kolyshkin [email protected]