From 2831fb559528a679ebe28e0bc11cc7d9dd74bbc5 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 5 Feb 2021 17:40:28 +1100 Subject: [PATCH] cgroup2: devices: handle eBPF skipping more correctly In the past we incorrectly handled eBPF errors in two ways: 1. We would only ignore errors if there was an allow rule in the list (this doesn't make sense because for security purposes we only care if a *deny* rule is being ignored). Arguably this is a security flaw but you would only get an error from bpf(2) in rare cases, and thus is not a big enough deal to go through security review. 2. If we were in a rootless container we would still return an error even though bpf(2) is blocked for rootless containers. Signed-off-by: Aleksa Sarai --- libcontainer/cgroups/fs2/devices.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/libcontainer/cgroups/fs2/devices.go b/libcontainer/cgroups/fs2/devices.go index 4c793a1cc..99559fec5 100644 --- a/libcontainer/cgroups/fs2/devices.go +++ b/libcontainer/cgroups/fs2/devices.go @@ -7,6 +7,8 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups/ebpf/devicefilter" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/devices" + "github.com/opencontainers/runc/libcontainer/system" + "github.com/pkg/errors" "golang.org/x/sys/unix" ) @@ -26,11 +28,26 @@ func isRWM(perms devices.Permissions) bool { return r && w && m } -// the logic is from crun -// https://github.com/containers/crun/blob/0.10.2/src/libcrun/cgroup.c#L1644-L1652 +// This is similar to the logic applied in crun for handling errors from bpf(2) +// . func canSkipEBPFError(cgroup *configs.Cgroup) bool { + // If we're running in a user namespace we can ignore eBPF rules because we + // usually cannot use bpf(2), as well as rootless containers usually don't + // have the necessary privileges to mknod(2) device inodes or access + // host-level instances (though ideally we would be blocking device access + // for rootless containers anyway). + if system.RunningInUserNS() { + return true + } + + // We cannot ignore an eBPF load error if any rule if is a block rule or it + // doesn't permit all access modes. + // + // NOTE: This will sometimes trigger in cases where access modes are split + // between different rules but to handle this correctly would require + // using ".../libcontainer/cgroup/devices".Emulator. for _, dev := range cgroup.Resources.Devices { - if dev.Allow || !isRWM(dev.Permissions) { + if !dev.Allow || !isRWM(dev.Permissions) { return false } }