From 42ffe74a1ffa627a0dfa8f084fd27d5e753e0d14 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Mon, 3 Oct 2022 11:16:53 -0700 Subject: [PATCH] Fixes inability to use /dev/null when inside a container This is a forward port of https://github.com/opencontainers/runc/pull/3620 The original code depended on the origin filesystem to have /dev/{block,char} populated. This is done by udev normally and while is very common non-containerized systemd installs, it's very easy to start systemd in a container created by runc itself and not have /dev/{block,char} populated. When this occurs, the following error output is observed: $ docker run hello-world docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error reopening /dev/null inside container: open /dev/null: operation not permitted: unknown. /dev/null can't be opened because it was not added to the deviceAllowList, as there was no /dev/char directory. The change here utilizes the fact that when sysfs in in use, there is a /sys/dev/{block,char} that is kernel maintained that we can check. Signed-off-by: Evan Phoenix --- devices/systemd.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/devices/systemd.go b/devices/systemd.go index 6594d9c0..eb5ecb06 100644 --- a/devices/systemd.go +++ b/devices/systemd.go @@ -133,8 +133,18 @@ func systemdProperties(r *configs.Resources) ([]systemdDbus.Property, error) { // 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 + // Also check /sys/dev so that we don't depend on /dev/{block,char} + // being populated. (/dev/{block,char} is populated by udev, which + // isn't strictly required for systemd). Ironically, this happens most + // easily when starting containerd within a runc created container + // itself. + + // We don't bother with securejoin here because we create entry.Path + // right above here, so we know it's safe. + if _, err := os.Stat("/sys" + entry.Path); err != nil { + logrus.Warnf("skipping device %s for systemd: %s", entry.Path, err) + continue + } } } deviceAllowList = append(deviceAllowList, entry)