From c8f262fec912ed6c0d2d97b4249756f88baf552a Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 27 Nov 2023 20:38:15 -0500 Subject: [PATCH] Use idtools.SafeChown and SafeLchown everywhere If we get an error chowning a file or directory to a UID/GID pair for something like ENOSUP or EPERM, then we should ignore as long as the UID/GID pair on disk is correct. Fixes: https://github.com/containers/podman/issues/20801 [NO NEW TESTS NEEDED] Since this is difficult to test and existing tests should be sufficient to ensure no regression. Signed-off-by: Daniel J Walsh --- libpod/container_internal.go | 12 ++++++------ libpod/container_internal_common.go | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 90f9234aa3..387aea7b8e 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -535,11 +535,11 @@ func (c *Container) setupStorage(ctx context.Context) error { c.state.RunDir = containerInfo.RunDir if len(c.config.IDMappings.UIDMap) != 0 || len(c.config.IDMappings.GIDMap) != 0 { - if err := os.Chown(containerInfo.RunDir, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(containerInfo.RunDir, c.RootUID(), c.RootGID()); err != nil { return err } - if err := os.Chown(containerInfo.Dir, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(containerInfo.Dir, c.RootUID(), c.RootGID()); err != nil { return err } } @@ -681,7 +681,7 @@ func (c *Container) refresh() error { if err := os.MkdirAll(root, 0755); err != nil { return fmt.Errorf("creating userNS tmpdir for container %s: %w", c.ID(), err) } - if err := os.Chown(root, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(root, c.RootUID(), c.RootGID()); err != nil { return err } } @@ -1578,7 +1578,7 @@ func (c *Container) mountStorage() (_ string, deferredErr error) { if err := c.mountSHM(shmOptions); err != nil { return "", err } - if err := os.Chown(c.config.ShmDir, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(c.config.ShmDir, c.RootUID(), c.RootGID()); err != nil { return "", fmt.Errorf("failed to chown %s: %w", c.config.ShmDir, err) } defer func() { @@ -2325,7 +2325,7 @@ func (c *Container) mount() (string, error) { if err != nil { return "", fmt.Errorf("resolving storage path for container %s: %w", c.ID(), err) } - if err := os.Chown(mountPoint, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(mountPoint, c.RootUID(), c.RootGID()); err != nil { return "", fmt.Errorf("cannot chown %s to %d:%d: %w", mountPoint, c.RootUID(), c.RootGID(), err) } return mountPoint, nil @@ -2508,7 +2508,7 @@ func (c *Container) extractSecretToCtrStorage(secr *ContainerSecret) error { if err != nil { return fmt.Errorf("unable to create %s: %w", secretFile, err) } - if err := os.Lchown(secretFile, int(hostUID), int(hostGID)); err != nil { + if err := idtools.SafeLchown(secretFile, int(hostUID), int(hostGID)); err != nil { return err } if err := os.Chmod(secretFile, os.FileMode(secr.Mode)); err != nil { diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index fc5978942c..b66268765b 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -795,7 +795,7 @@ func (c *Container) resolveWorkDir() error { if err != nil { return fmt.Errorf("looking up %s inside of the container %s: %w", c.User(), c.ID(), err) } - if err := os.Chown(resolvedWorkdir, int(uid), int(gid)); err != nil { + if err := idtools.SafeChown(resolvedWorkdir, int(uid), int(gid)); err != nil { return fmt.Errorf("chowning container %s workdir to container root: %w", c.ID(), err) } @@ -1820,7 +1820,7 @@ func (c *Container) mountIntoRootDirs(mountName string, mountPath string) error // Make standard bind mounts to include in the container func (c *Container) makeBindMounts() error { - if err := os.Chown(c.state.RunDir, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(c.state.RunDir, c.RootUID(), c.RootGID()); err != nil { return fmt.Errorf("cannot chown run directory: %w", err) } @@ -2285,7 +2285,7 @@ func (c *Container) addHosts() error { // It will also add the path to the container bind mount map. // source is the path on the host, dest is the path in the container. func (c *Container) bindMountRootFile(source, dest string) error { - if err := os.Chown(source, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(source, c.RootUID(), c.RootGID()); err != nil { return err } if err := c.relabel(source, c.MountLabel(), false); err != nil { @@ -2827,7 +2827,7 @@ func (c *Container) createSecretMountDir(runPath string) error { if err := c.relabel(src, c.config.MountLabel, false); err != nil { return err } - if err := os.Chown(src, c.RootUID(), c.RootGID()); err != nil { + if err := idtools.SafeChown(src, c.RootUID(), c.RootGID()); err != nil { return err } c.state.BindMounts[filepath.Join(runPath, "secrets")] = src @@ -2886,7 +2886,7 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { return err } - if err := os.Lchown(mountPoint, uid, gid); err != nil { + if err := idtools.SafeLchown(mountPoint, uid, gid); err != nil { return err } @@ -2895,7 +2895,7 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { st, err := os.Lstat(filepath.Join(c.state.Mountpoint, v.Dest)) if err == nil { if stat, ok := st.Sys().(*syscall.Stat_t); ok { - if err := os.Lchown(mountPoint, int(stat.Uid), int(stat.Gid)); err != nil { + if err := idtools.SafeLchown(mountPoint, int(stat.Uid), int(stat.Gid)); err != nil { return err } }