mirror of
https://github.com/opencontainers/runc.git
synced 2026-07-11 06:03:57 +08:00
Merge pull request #4334 from cyphar/1.1-rootfs-mountfd
[1.1] rootfs: fix 'can we mount on top of /proc' check
This commit is contained in:
@@ -1300,7 +1300,7 @@ func (c *linuxContainer) makeCriuRestoreMountpoints(m *configs.Mount) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkProcMount(c.config.Rootfs, dest, ""); err != nil {
|
||||
if err := checkProcMount(c.config.Rootfs, dest, m, ""); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := os.MkdirAll(dest, 0o755); err != nil {
|
||||
|
||||
@@ -244,7 +244,7 @@ func prepareBindMount(m *configs.Mount, rootfs string, mountFd *int) error {
|
||||
if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkProcMount(rootfs, dest, source); err != nil {
|
||||
if err := checkProcMount(rootfs, dest, m, source); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := createIfNotExists(dest, stat.IsDir()); err != nil {
|
||||
@@ -509,7 +509,7 @@ func mountToRootfs(m *configs.Mount, c *mountConfig) error {
|
||||
}
|
||||
return mountCgroupV1(m, c)
|
||||
default:
|
||||
if err := checkProcMount(rootfs, dest, m.Source); err != nil {
|
||||
if err := checkProcMount(rootfs, dest, m, m.Source); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := os.MkdirAll(dest, 0o755); err != nil {
|
||||
@@ -557,11 +557,17 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
|
||||
return binds, nil
|
||||
}
|
||||
|
||||
// checkProcMount checks to ensure that the mount destination is not over the top of /proc.
|
||||
// dest is required to be an abs path and have any symlinks resolved before calling this function.
|
||||
// Taken from <include/linux/proc_ns.h>. If a file is on a filesystem of type
|
||||
// PROC_SUPER_MAGIC, we're guaranteed that only the root of the superblock will
|
||||
// have this inode number.
|
||||
const procRootIno = 1
|
||||
|
||||
// checkProcMount checks to ensure that the mount destination is not over the
|
||||
// top of /proc. dest is required to be an abs path and have any symlinks
|
||||
// resolved before calling this function.
|
||||
//
|
||||
// if source is nil, don't stat the filesystem. This is used for restore of a checkpoint.
|
||||
func checkProcMount(rootfs, dest, source string) error {
|
||||
// source is "" when doing criu restores.
|
||||
func checkProcMount(rootfs, dest string, m *configs.Mount, source string) error {
|
||||
const procPath = "/proc"
|
||||
path, err := filepath.Rel(filepath.Join(rootfs, procPath), dest)
|
||||
if err != nil {
|
||||
@@ -572,18 +578,39 @@ func checkProcMount(rootfs, dest, source string) error {
|
||||
return nil
|
||||
}
|
||||
if path == "." {
|
||||
// an empty source is pasted on restore
|
||||
// Skip this check for criu restores.
|
||||
// NOTE: This is a special case kept from the original implementation,
|
||||
// only present for the 1.1.z branch to avoid any possible breakage in
|
||||
// a patch release. This check was removed in commit cdff09ab8751
|
||||
// ("rootfs: fix 'can we mount on top of /proc' check") in 1.2, because
|
||||
// it doesn't make sense with the new IsBind()-based checks.
|
||||
if source == "" {
|
||||
return nil
|
||||
}
|
||||
// only allow a mount on-top of proc if it's source is "proc"
|
||||
isproc, err := isProc(source)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// pass if the mount is happening on top of /proc and the source of
|
||||
// the mount is a proc filesystem
|
||||
if isproc {
|
||||
// Only allow bind-mounts on top of /proc, and only if the source is a
|
||||
// procfs mount.
|
||||
if m.IsBind() {
|
||||
var fsSt unix.Statfs_t
|
||||
if err := unix.Statfs(source, &fsSt); err != nil {
|
||||
return &os.PathError{Op: "statfs", Path: source, Err: err}
|
||||
}
|
||||
if fsSt.Type == unix.PROC_SUPER_MAGIC {
|
||||
var uSt unix.Stat_t
|
||||
if err := unix.Stat(source, &uSt); err != nil {
|
||||
return &os.PathError{Op: "stat", Path: source, Err: err}
|
||||
}
|
||||
if uSt.Ino != procRootIno {
|
||||
// We cannot error out in this case, because we've
|
||||
// supported these kinds of mounts for a long time.
|
||||
// However, we would expect users to bind-mount the root of
|
||||
// a real procfs on top of /proc in the container. We might
|
||||
// want to block this in the future.
|
||||
logrus.Warnf("bind-mount %v (source %v) is of type procfs but is not the root of a procfs (inode %d). Future versions of runc might block this configuration -- please report an issue to <https://github.com/opencontainers/runc> if you see this warning.", dest, source, uSt.Ino)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
} else if m.Device == "proc" {
|
||||
// Fresh procfs-type mounts are always safe to mount on top of /proc.
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("%q cannot be mounted because it is not of type proc", dest)
|
||||
@@ -617,14 +644,6 @@ func checkProcMount(rootfs, dest, source string) error {
|
||||
return fmt.Errorf("%q cannot be mounted because it is inside /proc", dest)
|
||||
}
|
||||
|
||||
func isProc(path string) (bool, error) {
|
||||
var s unix.Statfs_t
|
||||
if err := unix.Statfs(path, &s); err != nil {
|
||||
return false, &os.PathError{Op: "statfs", Path: path, Err: err}
|
||||
}
|
||||
return s.Type == unix.PROC_SUPER_MAGIC, nil
|
||||
}
|
||||
|
||||
func setupDevSymlinks(rootfs string) error {
|
||||
links := [][2]string{
|
||||
{"/proc/self/fd", "/dev/fd"},
|
||||
|
||||
@@ -3,52 +3,129 @@ package libcontainer
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"golang.org/x/sys/unix"
|
||||
|
||||
"github.com/opencontainers/runc/libcontainer/configs"
|
||||
)
|
||||
|
||||
func TestCheckMountDestOnProc(t *testing.T) {
|
||||
func TestCheckMountDestInProc(t *testing.T) {
|
||||
m := &configs.Mount{
|
||||
Destination: "/proc/sys",
|
||||
Source: "/proc/sys",
|
||||
Device: "bind",
|
||||
Flags: unix.MS_BIND,
|
||||
}
|
||||
dest := "/rootfs/proc/sys"
|
||||
err := checkProcMount("/rootfs", dest, "")
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err == nil {
|
||||
t.Fatal("destination inside proc should return an error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckMountDestOnProcChroot(t *testing.T) {
|
||||
func TestCheckProcMountOnProc(t *testing.T) {
|
||||
m := &configs.Mount{
|
||||
Destination: "/proc",
|
||||
Source: "foo",
|
||||
Device: "proc",
|
||||
}
|
||||
dest := "/rootfs/proc/"
|
||||
err := checkProcMount("/rootfs", dest, "/proc")
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err != nil {
|
||||
t.Fatal("destination inside proc when using chroot should not return an error")
|
||||
t.Fatalf("procfs type mount on /proc should not return an error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckBindMountOnProc(t *testing.T) {
|
||||
m := &configs.Mount{
|
||||
Destination: "/proc",
|
||||
Source: "/proc/self",
|
||||
Device: "bind",
|
||||
Flags: unix.MS_BIND,
|
||||
}
|
||||
dest := "/rootfs/proc/"
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err != nil {
|
||||
t.Fatalf("bind-mount of procfs on top of /proc should not return an error (for now): %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckTrickyMountOnProc(t *testing.T) {
|
||||
// Make a non-bind mount that looks like a bit like a bind-mount.
|
||||
m := &configs.Mount{
|
||||
Destination: "/proc",
|
||||
Source: "/proc",
|
||||
Device: "overlay",
|
||||
Data: "lowerdir=/tmp/fakeproc,upperdir=/tmp/fakeproc2,workdir=/tmp/work",
|
||||
}
|
||||
dest := "/rootfs/proc/"
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err == nil {
|
||||
t.Fatalf("dodgy overlayfs mount on top of /proc should return an error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckTrickyBindMountOnProc(t *testing.T) {
|
||||
// Make a bind mount that looks like it might be a procfs mount.
|
||||
m := &configs.Mount{
|
||||
Destination: "/proc",
|
||||
Source: "/sys",
|
||||
Device: "proc",
|
||||
Flags: unix.MS_BIND,
|
||||
}
|
||||
dest := "/rootfs/proc/"
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err == nil {
|
||||
t.Fatalf("dodgy bind-mount on top of /proc should return an error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckMountDestInSys(t *testing.T) {
|
||||
m := &configs.Mount{
|
||||
Destination: "/sys/fs/cgroup",
|
||||
Source: "tmpfs",
|
||||
Device: "tmpfs",
|
||||
}
|
||||
dest := "/rootfs//sys/fs/cgroup"
|
||||
err := checkProcMount("/rootfs", dest, "")
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err != nil {
|
||||
t.Fatal("destination inside /sys should not return an error")
|
||||
t.Fatalf("destination inside /sys should not return an error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckMountDestFalsePositive(t *testing.T) {
|
||||
m := &configs.Mount{
|
||||
Destination: "/sysfiles/fs/cgroup",
|
||||
Source: "tmpfs",
|
||||
Device: "tmpfs",
|
||||
}
|
||||
dest := "/rootfs/sysfiles/fs/cgroup"
|
||||
err := checkProcMount("/rootfs", dest, "")
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckMountDestNsLastPid(t *testing.T) {
|
||||
m := &configs.Mount{
|
||||
Destination: "/proc/sys/kernel/ns_last_pid",
|
||||
Source: "lxcfs",
|
||||
Device: "fuse.lxcfs",
|
||||
}
|
||||
dest := "/rootfs/proc/sys/kernel/ns_last_pid"
|
||||
err := checkProcMount("/rootfs", dest, "/proc")
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err != nil {
|
||||
t.Fatal("/proc/sys/kernel/ns_last_pid should not return an error")
|
||||
t.Fatalf("/proc/sys/kernel/ns_last_pid should not return an error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckCryptoFipsEnabled(t *testing.T) {
|
||||
m := &configs.Mount{
|
||||
Destination: "/proc/sys/crypto/fips_enabled",
|
||||
Source: "tmpfs",
|
||||
Device: "tmpfs",
|
||||
}
|
||||
dest := "/rootfs/proc/sys/crypto/fips_enabled"
|
||||
err := checkProcMount("/rootfs", dest, "/proc")
|
||||
err := checkProcMount("/rootfs", dest, m, m.Source)
|
||||
if err != nil {
|
||||
t.Fatalf("/proc/sys/crypto/fips_enabled should not return an error: %v", err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user