94 lines
3.6 KiB
Diff
94 lines
3.6 KiB
Diff
From 204c673cced91b03aa337c804ab8b31a452c6777 Mon Sep 17 00:00:00 2001
|
|
From: Kir Kolyshkin <kolyshkin@gmail.com>
|
|
Date: Wed, 10 Aug 2022 17:09:23 -0700
|
|
Subject: [PATCH] [1.1] fix failed exec after systemctl daemon-reload
|
|
|
|
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 7219387eb7db69b 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 58b1374f0ad98cc9390adc43dc22ddbb4f84d72e
|
|
to release-1.1 branch.
|
|
|
|
Fixes: https://github.com/opencontainers/runc/issues/3551
|
|
Fixes: 7219387eb7db69b4dae740c9d37d973d9a735801
|
|
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
|
---
|
|
libcontainer/cgroups/systemd/common.go | 16 +++++++++-------
|
|
tests/integration/dev.bats | 16 ++++++++++++++++
|
|
2 files changed, 25 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go
|
|
index 5a68a3cf39..45744c15c0 100644
|
|
--- a/libcontainer/cgroups/systemd/common.go
|
|
+++ b/libcontainer/cgroups/systemd/common.go
|
|
@@ -288,14 +288,16 @@ func generateDeviceProperties(r *configs.Resources) ([]systemdDbus.Property, err
|
|
case devices.CharDevice:
|
|
entry.Path = fmt.Sprintf("/dev/char/%d:%d", rule.Major, rule.Minor)
|
|
}
|
|
+ // systemd will issue a warning if the path we give here doesn't exist.
|
|
+ // Since all of this logic is best-effort anyway (we manually set these
|
|
+ // rules separately to systemd) we can safely skip entries that don't
|
|
+ // have a corresponding path.
|
|
+ if _, err := os.Stat(entry.Path); err != nil {
|
|
+ logrus.Debugf("skipping device %s for systemd: %s", entry.Path, err)
|
|
+ continue
|
|
+ }
|
|
}
|
|
- // systemd will issue a warning if the path we give here doesn't exist.
|
|
- // Since all of this logic is best-effort anyway (we manually set these
|
|
- // rules separately to systemd) we can safely skip entries that don't
|
|
- // have a corresponding path.
|
|
- if _, err := os.Stat(entry.Path); err == nil {
|
|
- deviceAllowList = append(deviceAllowList, entry)
|
|
- }
|
|
+ deviceAllowList = append(deviceAllowList, entry)
|
|
}
|
|
|
|
properties = append(properties, newProp("DeviceAllow", deviceAllowList))
|
|
diff --git a/tests/integration/dev.bats b/tests/integration/dev.bats
|
|
index 01f6778598..243315717a 100644
|
|
--- a/tests/integration/dev.bats
|
|
+++ b/tests/integration/dev.bats
|
|
@@ -128,3 +128,19 @@ function teardown() {
|
|
runc exec test_allow_block sh -c 'fdisk -l '"$device"''
|
|
[ "$status" -eq 0 ]
|
|
}
|
|
+
|
|
+# https://github.com/opencontainers/runc/issues/3551
|
|
+@test "runc exec vs systemctl daemon-reload" {
|
|
+ requires systemd root
|
|
+
|
|
+ runc run -d --console-socket "$CONSOLE_SOCKET" test_exec
|
|
+ [ "$status" -eq 0 ]
|
|
+
|
|
+ runc exec -t test_exec sh -c "ls -l /proc/self/fd/0; echo 123"
|
|
+ [ "$status" -eq 0 ]
|
|
+
|
|
+ systemctl daemon-reload
|
|
+
|
|
+ runc exec -t test_exec sh -c "ls -l /proc/self/fd/0; echo 123"
|
|
+ [ "$status" -eq 0 ]
|
|
+}
|