From 2e2ecf29ff6f1fd32f819c5a168872bb90d454f8 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 27 Sep 2023 16:32:31 -0700 Subject: [PATCH] libct: use chmod instead of umask Umask is problematic for Go programs as it affects other goroutines (see [1] for more details). Instead of using it, let's just prop up with Chmod. Note this patch misses the MkdirAll call in createDeviceNode. Since the runtime spec does not say anything about creating intermediary directories for device nodes, let's assume that doing it via mkdir with the current umask set is sufficient (if not, we have to reimplement MkdirAll from scratch, with added call to os.Chmod). [1] https://github.com/opencontainers/runc/pull/3563#discussion_r990293788 Signed-off-by: Kir Kolyshkin --- libcontainer/console_linux.go | 6 ++++-- libcontainer/container_linux.go | 7 ++++--- libcontainer/rootfs_linux.go | 7 ++++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/libcontainer/console_linux.go b/libcontainer/console_linux.go index d2b9cf59438..e506853e45a 100644 --- a/libcontainer/console_linux.go +++ b/libcontainer/console_linux.go @@ -9,13 +9,15 @@ import ( // mount initializes the console inside the rootfs mounting with the specified mount label // and applying the correct ownership of the console. func mountConsole(slavePath string) error { - oldMask := unix.Umask(0o000) - defer unix.Umask(oldMask) f, err := os.Create("/dev/console") if err != nil && !os.IsExist(err) { return err } if f != nil { + // Ensure permission bits (can be different because of umask). + if err := f.Chmod(0o666); err != nil { + return err + } f.Close() } return mount(slavePath, "/dev/console", "bind", unix.MS_BIND, "") diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index ae5d4fb46b4..d1a7b6055f8 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -413,12 +413,13 @@ func (c *Container) createExecFifo() error { if _, err := os.Stat(fifoName); err == nil { return fmt.Errorf("exec fifo %s already exists", fifoName) } - oldMask := unix.Umask(0o000) if err := unix.Mkfifo(fifoName, 0o622); err != nil { - unix.Umask(oldMask) + return &os.PathError{Op: "mkfifo", Path: fifoName, Err: err} + } + // Ensure permission bits (can be different because of umask). + if err := os.Chmod(fifoName, 0o622); err != nil { return err } - unix.Umask(oldMask) return os.Chown(fifoName, rootuid, rootgid) } diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 88ecb287c11..3145299d33d 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -704,7 +704,6 @@ func reOpenDevNull() error { // Create the device nodes in the container. func createDevices(config *configs.Config) error { useBindMount := userns.RunningInUserNS() || config.Namespaces.Contains(configs.NEWUSER) - oldMask := unix.Umask(0o000) for _, node := range config.Devices { // The /dev/ptmx device is setup by setupPtmx() @@ -715,11 +714,9 @@ func createDevices(config *configs.Config) error { // containers running in a user namespace are not allowed to mknod // devices so we can just bind mount it from the host. if err := createDeviceNode(config.Rootfs, node, useBindMount); err != nil { - unix.Umask(oldMask) return err } } - unix.Umask(oldMask) return nil } @@ -782,6 +779,10 @@ func mknodDevice(dest string, node *devices.Device) error { if err := unix.Mknod(dest, uint32(fileMode), int(dev)); err != nil { return &os.PathError{Op: "mknod", Path: dest, Err: err} } + // Ensure permission bits (can be different because of umask). + if err := os.Chmod(dest, fileMode); err != nil { + return err + } return os.Chown(dest, int(node.Uid), int(node.Gid)) }