Merge pull request #15076 from mheon/bump_420_rc2

Bump to v4.2.0-RC2

15 files changed, 91 insertions, 165 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index 742eb6d3e..2ff4bfe08 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -555,7 +555,7 @@ func (c *Container) WaitForExit(ctx context.Context, pollInterval time.Duration)
 				// The container never ran.
 				return true, 0, nil
 			}
-			return true, -1, err
+			return true, -1, fmt.Errorf("%w (container in state %s)", err, c.state.State)
 		}
 
 		return true, exitCode, nil
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 6a98466c2..7e330430c 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -22,12 +22,12 @@ import (
 	"github.com/containers/common/pkg/cgroups"
 	"github.com/containers/common/pkg/chown"
 	"github.com/containers/common/pkg/config"
+	"github.com/containers/common/pkg/hooks"
+	"github.com/containers/common/pkg/hooks/exec"
 	cutil "github.com/containers/common/pkg/util"
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/libpod/events"
 	"github.com/containers/podman/v4/pkg/ctime"
-	"github.com/containers/podman/v4/pkg/hooks"
-	"github.com/containers/podman/v4/pkg/hooks/exec"
 	"github.com/containers/podman/v4/pkg/lookup"
 	"github.com/containers/podman/v4/pkg/rootless"
 	"github.com/containers/podman/v4/pkg/selinux"
@@ -130,7 +130,7 @@ func (c *Container) bundlePath() string {
 	return c.config.StaticDir
 }
 
-// ControlSocketPath returns the path to the containers control socket for things like tty
+// ControlSocketPath returns the path to the container's control socket for things like tty
 // resizing
 func (c *Container) ControlSocketPath() string {
 	return filepath.Join(c.bundlePath(), "ctl")
@@ -513,8 +513,8 @@ func (c *Container) setupStorage(ctx context.Context) error {
 		return fmt.Errorf("error creating container storage: %w", containerInfoErr)
 	}
 
-	// only reconfig IDMappings if layer was mounted from storage
-	// if its a external overlay do not reset IDmappings
+	// Only reconfig IDMappings if layer was mounted from storage.
+	// If it's an external overlay do not reset IDmappings.
 	if !c.config.RootfsOverlay {
 		c.config.IDMappings.UIDMap = containerInfo.UIDMap
 		c.config.IDMappings.GIDMap = containerInfo.GIDMap
@@ -794,7 +794,7 @@ func (c *Container) save() error {
 }
 
 // Checks the container is in the right state, then initializes the container in preparation to start the container.
-// If recursive is true, each of the containers dependencies will be started.
+// If recursive is true, each of the container's dependencies will be started.
 // Otherwise, this function will return with error if there are dependencies of this container that aren't running.
 func (c *Container) prepareToStart(ctx context.Context, recursive bool) (retErr error) {
 	// Container must be created or stopped to be started
@@ -1255,7 +1255,7 @@ func (c *Container) stop(timeout uint) error {
 	// If the container is running in a PID Namespace, then killing the
 	// primary pid is enough to kill the container.  If it is not running in
 	// a pid namespace then the OCI Runtime needs to kill ALL processes in
-	// the containers cgroup in order to make sure the container is stopped.
+	// the container's cgroup in order to make sure the container is stopped.
 	all := !c.hasNamespace(spec.PIDNamespace)
 	// We can't use --all if Cgroups aren't present.
 	// Rootless containers with Cgroups v1 and NoCgroups are both cases
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 390e95258..a131ab367 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -184,11 +184,8 @@ func (c *Container) prepare() error {
 func (c *Container) isWorkDirSymlink(resolvedPath string) bool {
 	// We cannot create workdir since explicit --workdir is
 	// set in config but workdir could also be a symlink.
-	// If its a symlink lets check if resolved link is present
-	// on the container or not.
-
-	// If we can resolve symlink and resolved link is present on the container
-	// then return nil cause its a valid use-case.
+	// If it's a symlink, check if the resolved target is present in the container.
+	// If so, that's a valid use case: return nil.
 
 	maxSymLinks := 0
 	for {
@@ -2889,23 +2886,6 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) {
 		}
 	}
 
-	// Next, check if the container even has a /etc/passwd or /etc/group.
-	// If it doesn't we don't want to create them ourselves.
-	if needPasswd {
-		exists, err := c.checkFileExistsInRootfs("/etc/passwd")
-		if err != nil {
-			return "", "", err
-		}
-		needPasswd = exists
-	}
-	if needGroup {
-		exists, err := c.checkFileExistsInRootfs("/etc/group")
-		if err != nil {
-			return "", "", err
-		}
-		needGroup = exists
-	}
-
 	// If we don't need a /etc/passwd or /etc/group at this point we can
 	// just return.
 	if !needPasswd && !needGroup {
@@ -2950,7 +2930,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) {
 				return "", "", fmt.Errorf("error looking up location of container %s /etc/passwd: %w", c.ID(), err)
 			}
 
-			f, err := os.OpenFile(containerPasswd, os.O_APPEND|os.O_WRONLY, 0600)
+			f, err := os.OpenFile(containerPasswd, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
 			if err != nil {
 				return "", "", fmt.Errorf("container %s: %w", c.ID(), err)
 			}
@@ -2996,7 +2976,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) {
 				return "", "", fmt.Errorf("error looking up location of container %s /etc/group: %w", c.ID(), err)
 			}
 
-			f, err := os.OpenFile(containerGroup, os.O_APPEND|os.O_WRONLY, 0600)
+			f, err := os.OpenFile(containerGroup, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
 			if err != nil {
 				return "", "", fmt.Errorf("container %s: %w", c.ID(), err)
 			}
@@ -3115,26 +3095,6 @@ func (c *Container) cleanupOverlayMounts() error {
 	return overlay.CleanupContent(c.config.StaticDir)
 }
 
-// Check if a file exists at the given path in the container's root filesystem.
-// Container must already be mounted for this to be used.
-func (c *Container) checkFileExistsInRootfs(file string) (bool, error) {
-	checkPath, err := securejoin.SecureJoin(c.state.Mountpoint, file)
-	if err != nil {
-		return false, fmt.Errorf("cannot create path to container %s file %q: %w", c.ID(), file, err)
-	}
-	stat, err := os.Stat(checkPath)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return false, nil
-		}
-		return false, fmt.Errorf("container %s: %w", c.ID(), err)
-	}
-	if stat.IsDir() {
-		return false, nil
-	}
-	return true, nil
-}
-
 // Creates and mounts an empty dir to mount secrets into, if it does not already exist
 func (c *Container) createSecretMountDir() error {
 	src := filepath.Join(c.state.RunDir, "/run/secrets")
diff --git a/libpod/container_log.go b/libpod/container_log.go
index a9e0fe065..c49b54eb1 100644
--- a/libpod/container_log.go
+++ b/libpod/container_log.go
@@ -10,6 +10,7 @@ import (
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/libpod/events"
 	"github.com/containers/podman/v4/libpod/logs"
+	"github.com/nxadm/tail"
 	"github.com/nxadm/tail/watch"
 	"github.com/sirupsen/logrus"
 )
@@ -74,14 +75,19 @@ func (c *Container) readFromLogFile(ctx context.Context, options *logs.LogOption
 
 	go func() {
 		defer options.WaitGroup.Done()
-
-		for line := range t.Lines {
+		var line *tail.Line
+		var ok bool
+		for {
 			select {
 			case <-ctx.Done():
 				// the consumer has cancelled
+				t.Kill(errors.New("hangup by client"))
 				return
-			default:
-				// fallthrough
+			case line, ok = <-t.Lines:
+				if !ok {
+					// channel was closed
+					return
+				}
 			}
 			nll, err := logs.NewLogLine(line.Text)
 			if err != nil {
diff --git a/libpod/container_log_linux.go b/libpod/container_log_linux.go
index 0686caed2..7e95f2449 100644
--- a/libpod/container_log_linux.go
+++ b/libpod/container_log_linux.go
@@ -178,8 +178,13 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption
 				if !options.Follow || !containerCouldBeLogging {
 					return
 				}
-				// Sleep until something's happening on the journal.
-				journal.Wait(sdjournal.IndefiniteWait)
+
+				// journal.Wait() is blocking, this would cause the goroutine to hang forever
+				// if no more journal entries are generated and thus if the client
+				// has closed the connection in the meantime to leak memory.
+				// Waiting only 5 seconds makes sure we can check if the client closed in the
+				// meantime at least every 5 seconds.
+				journal.Wait(5 * time.Second)
 				continue
 			}
 			lastReadCursor = cursor
diff --git a/libpod/define/annotations.go b/libpod/define/annotations.go
index 8f5279981..580286d6a 100644
--- a/libpod/define/annotations.go
+++ b/libpod/define/annotations.go
@@ -135,6 +135,11 @@ const (
 	// creating a checkpoint image to specify the name of host distribution on
 	// which the checkpoint was created.
 	CheckpointAnnotationDistributionName = "io.podman.annotations.checkpoint.distribution.name"
+
+	// InitContainerType is used by play kube when playing a kube yaml to specify the type
+	// of the init container.
+	InitContainerType = "io.podman.annotations.init.container.type"
+
 	// MaxKubeAnnotation is the max length of annotations allowed by Kubernetes.
 	MaxKubeAnnotation = 63
 )
diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go
index ccc4ae00f..e6a34ba61 100644
--- a/libpod/define/container_inspect.go
+++ b/libpod/define/container_inspect.go
@@ -506,8 +506,8 @@ type InspectContainerHostConfig struct {
 	// CpuRealtimeRuntime is the length of time (in microseconds) allocated
 	// for realtime tasks within every CpuRealtimePeriod.
 	CpuRealtimeRuntime int64 `json:"CpuRealtimeRuntime"`
-	// CpusetCpus is the is the set of CPUs that the container will execute
-	// on. Formatted as `0-3` or `0,2`. Default (if unset) is all CPUs.
+	// CpusetCpus is the set of CPUs that the container will execute on.
+	// Formatted as `0-3` or `0,2`. Default (if unset) is all CPUs.
 	CpusetCpus string `json:"CpusetCpus"`
 	// CpusetMems is the set of memory nodes the container will use.
 	// Formatted as `0-3` or `0,2`. Default (if unset) is all memory nodes.
@@ -544,7 +544,7 @@ type InspectContainerHostConfig struct {
 	OomKillDisable bool `json:"OomKillDisable"`
 	// Init indicates whether the container has an init mounted into it.
 	Init bool `json:"Init,omitempty"`
-	// PidsLimit is the maximum number of PIDs what may be created within
+	// PidsLimit is the maximum number of PIDs that may be created within
 	// the container. 0, the default, indicates no limit.
 	PidsLimit int64 `json:"PidsLimit"`
 	// Ulimits is a set of ulimits that will be set within the container.
diff --git a/libpod/define/errors.go b/libpod/define/errors.go
index b858e1989..fd27e89de 100644
--- a/libpod/define/errors.go
+++ b/libpod/define/errors.go
@@ -205,7 +205,7 @@ var (
 	// Useful for potentially long running tasks.
 	ErrCanceled = errors.New("cancelled by user")
 
-	// ErrConmonVersionFormat is used when the expected versio-format of conmon
+	// ErrConmonVersionFormat is used when the expected version format of conmon
 	// has changed.
 	ErrConmonVersionFormat = "conmon version changed format"
 )
diff --git a/libpod/define/volume_inspect.go b/libpod/define/volume_inspect.go
index f731a8735..9279812da 100644
--- a/libpod/define/volume_inspect.go
+++ b/libpod/define/volume_inspect.go
@@ -45,7 +45,7 @@ type InspectVolumeData struct {
 	// GID is the GID that the volume was created with.
 	GID int `json:"GID,omitempty"`
 	// Anonymous indicates that the volume was created as an anonymous
-	// volume for a specific container, and will be be removed when any
+	// volume for a specific container, and will be removed when any
 	// container using it is removed.
 	Anonymous bool `json:"Anonymous,omitempty"`
 	// MountCount is the number of times this volume has been mounted.
diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go
index 0a0a768d0..16ef6504f 100644
--- a/libpod/events/journal_linux.go
+++ b/libpod/events/journal_linux.go
@@ -141,9 +141,18 @@ func (e EventJournalD) Read(ctx context.Context, options ReadOptions) error {
 			if !options.Stream || (len(options.Until) > 0 && time.Now().After(untilTime)) {
 				break
 			}
-			t := sdjournal.IndefiniteWait
+
+			// j.Wait() is blocking, this would cause the goroutine to hang forever
+			// if no more journal entries are generated and thus if the client
+			// has closed the connection in the meantime to leak memory.
+			// Waiting only 5 seconds makes sure we can check if the client closed in the
+			// meantime at least every 5 seconds.
+			t := 5 * time.Second
 			if len(options.Until) > 0 {
-				t = time.Until(untilTime)
+				until := time.Until(untilTime)
+				if until < t {
+					t = until
+				}
 			}
 			_ = j.Wait(t)
 			continue
diff --git a/libpod/events/logfile.go b/libpod/events/logfile.go
index 4dafd8600..c7dbf4850 100644
--- a/libpod/events/logfile.go
+++ b/libpod/events/logfile.go
@@ -108,23 +108,19 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error {
 			}
 		}()
 	}
-	funcDone := make(chan bool)
-	copy := true
-	go func() {
-		select {
-		case <-funcDone:
-			// Do nothing
-		case <-ctx.Done():
-			copy = false
-			t.Kill(errors.New("hangup by client"))
-		}
-	}()
-	for line := range t.Lines {
+	var line *tail.Line
+	var ok bool
+	for {
 		select {
 		case <-ctx.Done():
 			// the consumer has cancelled
+			t.Kill(errors.New("hangup by client"))
 			return nil
-		default:
+		case line, ok = <-t.Lines:
+			if !ok {
+				// channel was closed
+				return nil
+			}
 			// fallthrough
 		}
 
@@ -138,12 +134,10 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error {
 		default:
 			return fmt.Errorf("event type %s is not valid in %s", event.Type.String(), e.options.LogFilePath)
 		}
-		if copy && applyFilters(event, filterMap) {
+		if applyFilters(event, filterMap) {
 			options.EventChannel <- event
 		}
 	}
-	funcDone <- true
-	return nil
 }
 
 // String returns a string representation of the logger
diff --git a/libpod/events/nullout.go b/libpod/events/nullout.go
index 3eca9e8db..587a1b98b 100644
--- a/libpod/events/nullout.go
+++ b/libpod/events/nullout.go
@@ -19,7 +19,7 @@ func (e EventToNull) Read(ctx context.Context, options ReadOptions) error {
 }
 
 // NewNullEventer returns a new null eventer.  You should only do this for
-// the purposes on internal libpod testing.
+// the purposes of internal libpod testing.
 func NewNullEventer() Eventer {
 	return EventToNull{}
 }
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index 121e750f4..cb76de72c 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -133,7 +133,7 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
 			continue
 		}
 		foundPath = true
-		logrus.Tracef("found runtime %q", runtime.path)
+		logrus.Tracef("found runtime %q", path)
 		runtime.path = path
 		break
 	}
@@ -1388,7 +1388,7 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
 }
 
 func startCommand(cmd *exec.Cmd, ctr *Container) error {
-	// Make sure to unset the NOTIFY_SOCKET and reset if afterwards if needed.
+	// Make sure to unset the NOTIFY_SOCKET and reset it afterwards if needed.
 	switch ctr.config.SdNotifyMode {
 	case define.SdNotifyModeContainer, define.SdNotifyModeIgnore:
 		if ctr.notifySocket != "" {
@@ -1667,7 +1667,7 @@ func httpAttachNonTerminalCopy(container *net.UnixConn, http *bufio.ReadWriter,
 			// multiplexing by Conmon).
 			headerLen := uint32(numR - 1)
 			// Practically speaking, we could make this buf[0] - 1,
-			// but we need to validate it anyways...
+			// but we need to validate it anyway.
 			switch buf[0] {
 			case AttachPipeStdin:
 				headerBuf = makeHTTPAttachHeader(0, headerLen)
diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go
index 75ff24e41..b22a0dbb6 100644
--- a/libpod/runtime_pod_linux.go
+++ b/libpod/runtime_pod_linux.go
@@ -7,7 +7,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"path"
 	"path/filepath"
 	"strings"
@@ -18,7 +17,6 @@ import (
 	"github.com/containers/podman/v4/libpod/events"
 	"github.com/containers/podman/v4/pkg/rootless"
 	"github.com/containers/podman/v4/pkg/specgen"
-	runcconfig "github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/sirupsen/logrus"
 )
 
@@ -214,83 +212,37 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool,
 		return fmt.Errorf("pod %s contains containers and cannot be removed: %w", p.ID(), define.ErrCtrExists)
 	}
 
-	// Go through and lock all containers so we can operate on them all at
-	// once.
-	// First loop also checks that we are ready to go ahead and remove.
-	containersLocked := true
+	ctrNamedVolumes := make(map[string]*ContainerNamedVolume)
+
+	var removalErr error
 	for _, ctr := range ctrs {
-		ctrLock := ctr.lock
-		ctrLock.Lock()
-		defer func() {
-			if containersLocked {
+		err := func() error {
+			ctrLock := ctr.lock
+			ctrLock.Lock()
+			defer func() {
 				ctrLock.Unlock()
-			}
-		}()
+			}()
 
-		// If we're force-removing, no need to check status.
-		if force {
-			continue
-		}
-
-		// Sync all containers
-		if err := ctr.syncContainer(); err != nil {
-			return err
-		}
-
-		// Ensure state appropriate for removal
-		if err := ctr.checkReadyForRemoval(); err != nil {
-			return fmt.Errorf("pod %s has containers that are not ready to be removed: %w", p.ID(), err)
-		}
-	}
-
-	// We're going to be removing containers.
-	// If we are Cgroupfs cgroup driver, to avoid races, we need to hit
-	// the pod and conmon Cgroups with a PID limit to prevent them from
-	// spawning any further processes (particularly cleanup processes) which
-	// would prevent removing the Cgroups.
-	if p.runtime.config.Engine.CgroupManager == config.CgroupfsCgroupsManager {
-		// Get the conmon Cgroup
-		conmonCgroupPath := filepath.Join(p.state.CgroupPath, "conmon")
-		conmonCgroup, err := cgroups.Load(conmonCgroupPath)
-		if err != nil && err != cgroups.ErrCgroupDeleted && err != cgroups.ErrCgroupV1Rootless {
-			logrus.Errorf("Retrieving pod %s conmon cgroup %s: %v", p.ID(), conmonCgroupPath, err)
-		}
-
-		// New resource limits
-		resLimits := new(runcconfig.Resources)
-		resLimits.PidsLimit = 1 // Inhibit forks with very low pids limit
-
-		// Don't try if we failed to retrieve the cgroup
-		if err == nil {
-			if err := conmonCgroup.Update(resLimits); err != nil && !os.IsNotExist(err) {
-				logrus.Warnf("Error updating pod %s conmon cgroup PID limit: %v", p.ID(), err)
+			if err := ctr.syncContainer(); err != nil {
+				return err
 			}
-		}
-	}
-
-	var removalErr error
 
-	ctrNamedVolumes := make(map[string]*ContainerNamedVolume)
+			for _, vol := range ctr.config.NamedVolumes {
+				ctrNamedVolumes[vol.Name] = vol
+			}
 
-	// Second loop - all containers are good, so we should be clear to
-	// remove.
-	for _, ctr := range ctrs {
-		// Remove the container.
-		// Do NOT remove named volumes. Instead, we're going to build a
-		// list of them to be removed at the end, once the containers
-		// have been removed by RemovePodContainers.
-		for _, vol := range ctr.config.NamedVolumes {
-			ctrNamedVolumes[vol.Name] = vol
-		}
+			return r.removeContainer(ctx, ctr, force, false, true, timeout)
+		}()
 
-		if err := r.removeContainer(ctx, ctr, force, false, true, timeout); err != nil {
-			if removalErr == nil {
-				removalErr = err
-			} else {
-				logrus.Errorf("Removing container %s from pod %s: %v", ctr.ID(), p.ID(), err)
-			}
+		if removalErr == nil {
+			removalErr = err
+		} else {
+			logrus.Errorf("Removing container %s from pod %s: %v", ctr.ID(), p.ID(), err)
 		}
 	}
+	if removalErr != nil {
+		return removalErr
+	}
 
 	// Clear infra container ID before we remove the infra container.
 	// There is a potential issue if we don't do that, and removal is
@@ -326,12 +278,6 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool,
 		}
 	}
 
-	// let's unlock the containers so if there is any cleanup process, it can terminate its execution
-	for _, ctr := range ctrs {
-		ctr.lock.Unlock()
-	}
-	containersLocked = false
-
 	// Remove pod cgroup, if present
 	if p.state.CgroupPath != "" {
 		logrus.Debugf("Removing pod cgroup %s", p.state.CgroupPath)
diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go
index a751d75d2..1f354e41b 100644
--- a/libpod/runtime_volume_linux.go
+++ b/libpod/runtime_volume_linux.go
@@ -16,6 +16,7 @@ import (
 	"github.com/containers/podman/v4/libpod/events"
 	volplugin "github.com/containers/podman/v4/libpod/plugin"
 	"github.com/containers/storage/drivers/quota"
+	"github.com/containers/storage/pkg/idtools"
 	"github.com/containers/storage/pkg/stringid"
 	pluginapi "github.com/docker/go-plugins-helpers/volume"
 	"github.com/sirupsen/logrus"
@@ -31,7 +32,7 @@ func (r *Runtime) NewVolume(ctx context.Context, options ...VolumeCreateOption)
 
 // newVolume creates a new empty volume with the given options.
 // The createPluginVolume can be set to true to make it not create the volume in the volume plugin,
-// this is required for the UpdateVolumePlugins() function. If you are not sure set this to false.
+// this is required for the UpdateVolumePlugins() function. If you are not sure, set this to false.
 func (r *Runtime) newVolume(noCreatePluginVolume bool, options ...VolumeCreateOption) (_ *Volume, deferredErr error) {
 	volume := newVolume(r)
 	for _, option := range options {
@@ -101,14 +102,14 @@ func (r *Runtime) newVolume(noCreatePluginVolume bool, options ...VolumeCreateOp
 		if err := os.MkdirAll(volPathRoot, 0700); err != nil {
 			return nil, fmt.Errorf("creating volume directory %q: %w", volPathRoot, err)
 		}
-		if err := os.Chown(volPathRoot, volume.config.UID, volume.config.GID); err != nil {
+		if err := idtools.SafeChown(volPathRoot, volume.config.UID, volume.config.GID); err != nil {
 			return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", volPathRoot, volume.config.UID, volume.config.GID, err)
 		}
 		fullVolPath := filepath.Join(volPathRoot, "_data")
 		if err := os.MkdirAll(fullVolPath, 0755); err != nil {
 			return nil, fmt.Errorf("creating volume directory %q: %w", fullVolPath, err)
 		}
-		if err := os.Chown(fullVolPath, volume.config.UID, volume.config.GID); err != nil {
+		if err := idtools.SafeChown(fullVolPath, volume.config.UID, volume.config.GID); err != nil {
 			return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", fullVolPath, volume.config.UID, volume.config.GID, err)
 		}
 		if err := LabelVolumePath(fullVolPath); err != nil {
@@ -216,7 +217,7 @@ func (r *Runtime) UpdateVolumePlugins(ctx context.Context) *define.VolumeReload
 	for _, vol := range libpodVolumes {
 		if vol.UsesVolumeDriver() {
 			if _, ok := allPluginVolumes[vol.Name()]; !ok {
-				// The volume is no longer in the plugin, lets remove it from the libpod db.
+				// The volume is no longer in the plugin. Let's remove it from the libpod db.
 				if err := r.removeVolume(ctx, vol, false, nil, true); err != nil {
 					if errors.Is(err, define.ErrVolumeBeingUsed) {
 						// Volume is still used by at least one container. This is very bad,