Skip to content

Commit

Permalink
Fixes inability to use /dev/null when inside a container
Browse files Browse the repository at this point in the history
This is a forward port of opencontainers#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 <evan@phx.io>
  • Loading branch information
evanphx committed Oct 3, 2022
1 parent 967552c commit 87e308a
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions libcontainer/cgroups/devices/systemd.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

systemdDbus "github.com/coreos/go-systemd/v22/dbus"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/godbus/dbus/v5"
"github.com/sirupsen/logrus"

Expand Down Expand Up @@ -133,8 +134,21 @@ 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.
testPath, err := securejoin.SecureJoin("/sys", entry.Path)
if err != nil {
logrus.Errorf("error joining entry path: %s", err)
continue
}

if _, err := os.Stat(testPath); err != nil {
logrus.Warnf("skipping device %s for systemd: %s", entry.Path, err)
continue
}
}
}
deviceAllowList = append(deviceAllowList, entry)
Expand Down

0 comments on commit 87e308a

Please sign in to comment.