Skip to content

Commit 58b1374

Browse files
committed
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 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/runc#3551 Fixes: 7219387 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1 parent a98ad5e commit 58b1374

File tree

2 files changed

+25
-7
lines changed

2 files changed

+25
-7
lines changed

libcontainer/cgroups/devices/systemd.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,16 @@ func systemdProperties(r *configs.Resources) ([]systemdDbus.Property, error) {
128128
case devices.CharDevice:
129129
entry.Path = fmt.Sprintf("/dev/char/%d:%d", rule.Major, rule.Minor)
130130
}
131+
// systemd will issue a warning if the path we give here doesn't exist.
132+
// Since all of this logic is best-effort anyway (we manually set these
133+
// rules separately to systemd) we can safely skip entries that don't
134+
// have a corresponding path.
135+
if _, err := os.Stat(entry.Path); err != nil {
136+
logrus.Debugf("skipping device %s for systemd: %s", entry.Path, err)
137+
continue
138+
}
131139
}
132-
// systemd will issue a warning if the path we give here doesn't exist.
133-
// Since all of this logic is best-effort anyway (we manually set these
134-
// rules separately to systemd) we can safely skip entries that don't
135-
// have a corresponding path.
136-
if _, err := os.Stat(entry.Path); err == nil {
137-
deviceAllowList = append(deviceAllowList, entry)
138-
}
140+
deviceAllowList = append(deviceAllowList, entry)
139141
}
140142

141143
properties = append(properties, newProp("DeviceAllow", deviceAllowList))

tests/integration/dev.bats

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,19 @@ function teardown() {
128128
runc exec test_allow_block sh -c 'fdisk -l '"$device"''
129129
[ "$status" -eq 0 ]
130130
}
131+
132+
# https://github.com/opencontainers/runc/issues/3551
133+
@test "runc exec vs systemctl daemon-reload" {
134+
requires systemd root
135+
136+
runc run -d --console-socket "$CONSOLE_SOCKET" test_exec
137+
[ "$status" -eq 0 ]
138+
139+
runc exec -t test_exec sh -c "ls -l /proc/self/fd/0; echo 123"
140+
[ "$status" -eq 0 ]
141+
142+
systemctl daemon-reload
143+
144+
runc exec -t test_exec sh -c "ls -l /proc/self/fd/0; echo 123"
145+
[ "$status" -eq 0 ]
146+
}

0 commit comments

Comments
 (0)