[1.4] Handle os.Is* wrapped errors correctly

.golangci.yml

@@ -37,6 +37,12 @@ linters:
         # going to be tricked into overwriting host files.
         - pattern: ^os\.Create$
           pkg: ^os$
+        # os.Is* error checking functions predate errors.Is. Therefore, they
+        # only support errors returned by the os package and subtly fail
+        # to deal with other wrapped error types.
+        # New code should use errors.Is(err, error-type) instead.
+        - pattern: ^os\.Is(Exist|NotExist|Permission|Timeout)$
+          pkg: ^os$
       analyze-types: true
   exclusions:
     rules:

libcontainer/configs/validate/validator.go

@@ -142,7 +142,7 @@ func security(config *configs.Config) error {
 
 func namespaces(config *configs.Config) error {
 	if config.Namespaces.Contains(configs.NEWUSER) {
-		if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
+		if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) {
 			return errors.New("user namespaces aren't enabled in the kernel")
 		}
 		hasPath := config.Namespaces.PathOf(configs.NEWUSER) != ""
@@ -160,13 +160,13 @@ func namespaces(config *configs.Config) error {
 	}
 
 	if config.Namespaces.Contains(configs.NEWCGROUP) {
-		if _, err := os.Stat("/proc/self/ns/cgroup"); os.IsNotExist(err) {
+		if _, err := os.Stat("/proc/self/ns/cgroup"); errors.Is(err, os.ErrNotExist) {
 			return errors.New("cgroup namespaces aren't enabled in the kernel")
 		}
 	}
 
 	if config.Namespaces.Contains(configs.NEWTIME) {
-		if _, err := os.Stat("/proc/self/timens_offsets"); os.IsNotExist(err) {
+		if _, err := os.Stat("/proc/self/timens_offsets"); errors.Is(err, os.ErrNotExist) {
 			return errors.New("time namespaces aren't enabled in the kernel")
 		}
 		hasPath := config.Namespaces.PathOf(configs.NEWTIME) != ""

libcontainer/configs/validate/validator_test.go

@@ -1,6 +1,7 @@
 package validate
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
@@ -172,7 +173,7 @@ func TestValidateSecurityWithoutNEWNS(t *testing.T) {
 }
 
 func TestValidateUserNamespace(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires userns.")
 	}
 	config := &configs.Config{
@@ -206,7 +207,7 @@ func TestValidateUsernsMappingWithoutNamespace(t *testing.T) {
 }
 
 func TestValidateTimeNamespace(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/time"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/time"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires timens.")
 	}
 	config := &configs.Config{
@@ -225,7 +226,7 @@ func TestValidateTimeNamespace(t *testing.T) {
 }
 
 func TestValidateTimeNamespaceWithBothPathAndTimeOffset(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/time"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/time"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires timens.")
 	}
 	config := &configs.Config{
@@ -1020,7 +1021,7 @@ func TestValidateNetDevices(t *testing.T) {
 }
 
 func TestValidateUserSysctlWithUserNamespace(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires userns.")
 	}
 	config := &configs.Config{

libcontainer/container_linux.go

@@ -295,7 +295,7 @@ func handleFifoResult(result openResult) error {
 		return err
 	}
 	err := os.Remove(f.Name())
-	if err == nil || os.IsNotExist(err) {
+	if err == nil || errors.Is(err, os.ErrNotExist) {
 		return nil
 	}
 	return err

libcontainer/criu_linux.go

@@ -124,7 +124,7 @@ func (c *Container) addMaskPaths(req *criurpc.CriuReq) error {
 	for _, path := range c.config.MaskPaths {
 		fi, err := os.Stat(fmt.Sprintf("/proc/%d/root/%s", c.initProcess.pid(), path))
 		if err != nil {
-			if os.IsNotExist(err) {
+			if errors.Is(err, os.ErrNotExist) {
 				continue
 			}
 			return err
@@ -305,7 +305,7 @@ func (c *Container) Checkpoint(criuOpts *CriuOpts) error {
 
 	// Since a container can be C/R'ed multiple times,
 	// the checkpoint directory may already exist.
-	if err := os.Mkdir(criuOpts.ImagesDirectory, 0o700); err != nil && !os.IsExist(err) {
+	if err := os.Mkdir(criuOpts.ImagesDirectory, 0o700); err != nil && !errors.Is(err, os.ErrExist) {
 		return err
 	}
 
@@ -340,7 +340,7 @@ func (c *Container) Checkpoint(criuOpts *CriuOpts) error {
 
 	// if criuOpts.WorkDirectory is not set, criu default is used.
 	if criuOpts.WorkDirectory != "" {
-		if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !os.IsExist(err) {
+		if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !errors.Is(err, os.ErrExist) {
 			return err
 		}
 		workDir, err := os.Open(criuOpts.WorkDirectory)
@@ -705,7 +705,7 @@ func (c *Container) Restore(process *Process, criuOpts *CriuOpts) error {
 	if criuOpts.WorkDirectory != "" {
 		// Since a container can be C/R'ed multiple times,
 		// the work directory may already exist.
-		if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !os.IsExist(err) {
+		if err := os.Mkdir(criuOpts.WorkDirectory, 0o700); err != nil && !errors.Is(err, os.ErrExist) {
 			return err
 		}
 		workDir, err := os.Open(criuOpts.WorkDirectory)
@@ -1156,7 +1156,7 @@ func (c *Container) criuNotifications(resp *criurpc.CriuResp, process *Process,
 			return err
 		}
 		if err := os.Remove(filepath.Join(c.stateDir, "checkpoint")); err != nil {
-			if !os.IsNotExist(err) {
+			if !errors.Is(err, os.ErrNotExist) {
 				logrus.Error(err)
 			}
 		}

libcontainer/devices/device_unix.go

@@ -98,7 +98,7 @@ func GetDevices(path string) ([]*Device, error) {
 			if errors.Is(err, ErrNotADevice) {
 				continue
 			}
-			if os.IsNotExist(err) {
+			if errors.Is(err, os.ErrNotExist) {
 				continue
 			}
 			return nil, err

libcontainer/factory_linux.go

@@ -51,7 +51,7 @@ func Create(root, id string, config *configs.Config) (*Container, error) {
 	}
 	if _, err := os.Stat(stateDir); err == nil {
 		return nil, ErrExist
-	} else if !os.IsNotExist(err) {
+	} else if !errors.Is(err, os.ErrNotExist) {
 		return nil, err
 	}
 
@@ -154,7 +154,7 @@ func loadState(root string) (*State, error) {
 	}
 	f, err := os.Open(stateFilePath)
 	if err != nil {
-		if os.IsNotExist(err) {
+		if errors.Is(err, os.ErrNotExist) {
 			return nil, ErrNotExist
 		}
 		return nil, err

libcontainer/init_linux.go

@@ -312,7 +312,7 @@ func finalizeNamespace(config *initConfig) error {
 		switch {
 		case err == nil:
 			doChdir = false
-		case os.IsPermission(err):
+		case errors.Is(err, os.ErrPermission):
 			// If we hit an EPERM, we should attempt again after setting up user.
 			// This will allow us to successfully chdir if the container user has access
 			// to the directory, but the user running runc does not.
@@ -478,7 +478,7 @@ func setupUser(config *initConfig) error {
 		setgroups, err = io.ReadAll(setgroupsFile)
 		_ = setgroupsFile.Close()
 	}
-	if err != nil && !os.IsNotExist(err) {
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
 		return err
 	}
 

libcontainer/integration/exec_test.go

@@ -3,6 +3,7 @@ package integration
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -1094,7 +1095,7 @@ func TestHook(t *testing.T) {
 
 	for _, hook := range []string{"prestart", "createRuntime", "poststart"} {
 		fi, err := os.Stat(filepath.Join(config.Rootfs, hook))
-		if err == nil || !os.IsNotExist(err) {
+		if err == nil || !errors.Is(err, os.ErrNotExist) {
 			t.Fatalf("expected file '%s to not exists, but it does", fi.Name())
 		}
 	}
@@ -1638,7 +1639,7 @@ func TestTmpfsCopyUp(t *testing.T) {
 }
 
 func TestCGROUPPrivate(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/cgroup"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/cgroup"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires cgroupns.")
 	}
 	if testing.Short() {
@@ -1658,7 +1659,7 @@ func TestCGROUPPrivate(t *testing.T) {
 }
 
 func TestCGROUPHost(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/cgroup"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/cgroup"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires cgroupns.")
 	}
 	if testing.Short() {

libcontainer/intelrdt/intelrdt.go

@@ -460,7 +460,7 @@ func (m *Manager) Apply(pid int) (err error) {
 		}
 	}
 
-	if err := os.Mkdir(path, 0o755); err != nil && !os.IsExist(err) {
+	if err := os.Mkdir(path, 0o755); err != nil && !errors.Is(err, os.ErrExist) {
 		return newLastCmdError(err)
 	}
 
@@ -470,7 +470,7 @@ func (m *Manager) Apply(pid int) (err error) {
 
 	// Create MON group
 	if monPath := m.GetMonPath(); monPath != "" {
-		if err := os.Mkdir(monPath, 0o755); err != nil && !os.IsExist(err) {
+		if err := os.Mkdir(monPath, 0o755); err != nil && !errors.Is(err, os.ErrExist) {
 			return newLastCmdError(err)
 		}
 		if err := WriteIntelRdtTasks(monPath, pid); err != nil {
@@ -493,14 +493,14 @@ func (m *Manager) Destroy() error {
 	if m.config.IntelRdt.ClosID == "" {
 		m.mu.Lock()
 		defer m.mu.Unlock()
-		if err := os.Remove(m.GetPath()); err != nil && !os.IsNotExist(err) {
+		if err := os.Remove(m.GetPath()); err != nil && !errors.Is(err, os.ErrNotExist) {
 			return err
 		}
 		m.path = ""
 	} else if monPath := m.GetMonPath(); monPath != "" {
 		// If ClosID is not specified the possible monintoring group was
 		// removed with the CLOS above.
-		if err := os.Remove(monPath); err != nil && !os.IsNotExist(err) {
+		if err := os.Remove(monPath); err != nil && !errors.Is(err, os.ErrNotExist) {
 			return err
 		}
 	}

libcontainer/notify_linux.go

@@ -51,7 +51,7 @@ func registerMemoryEvent(cgDir, evName, arg string) (<-chan struct{}, error) {
 			}
 			// When a cgroup is destroyed, an event is sent to eventfd.
 			// So if the control path is gone, return instead of notifying.
-			if _, err := os.Lstat(eventControlPath); os.IsNotExist(err) {
+			if _, err := os.Lstat(eventControlPath); errors.Is(err, os.ErrNotExist) {
 				return
 			}
 			ch <- struct{}{}

libcontainer/process_linux.go

@@ -1139,7 +1139,7 @@ func getPipeFds(pid int) ([]string, error) {
 			// Ignore permission errors, for rootless containers and other
 			// non-dumpable processes. if we can't get the fd for a particular
 			// file, there's not much we can do.
-			if os.IsPermission(err) {
+			if errors.Is(err, os.ErrPermission) {
 				continue
 			}
 			return fds, err

libcontainer/rootfs_linux.go

@@ -364,7 +364,7 @@ func mountCgroupV1(m mountEntry, c *mountConfig) error {
 			// symlink(2) is very dumb, it will just shove the path into
 			// the link and doesn't do any checks or relative path
 			// conversion. Also, don't error out if the cgroup already exists.
-			if err := os.Symlink(mc, filepath.Join(c.root, m.Destination, ss)); err != nil && !os.IsExist(err) {
+			if err := os.Symlink(mc, filepath.Join(c.root, m.Destination, ss)); err != nil && !errors.Is(err, os.ErrExist) {
 				return err
 			}
 		}
@@ -602,7 +602,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 			return err
 		}
 		if fi, err := os.Lstat(dest); err != nil {
-			if !os.IsNotExist(err) {
+			if !errors.Is(err, os.ErrNotExist) {
 				return err
 			}
 		} else if !fi.IsDir() {
@@ -899,7 +899,7 @@ func setupDevSymlinks(rootfs string) error {
 			src = link[0]
 			dst = filepath.Join(rootfs, link[1])
 		)
-		if err := os.Symlink(src, dst); err != nil && !os.IsExist(err) {
+		if err := os.Symlink(src, dst); err != nil && !errors.Is(err, os.ErrExist) {
 			return err
 		}
 	}
@@ -1109,7 +1109,7 @@ func setReadonly() error {
 
 func setupPtmx(config *configs.Config) error {
 	ptmx := filepath.Join(config.Rootfs, "dev/ptmx")
-	if err := os.Remove(ptmx); err != nil && !os.IsNotExist(err) {
+	if err := os.Remove(ptmx); err != nil && !errors.Is(err, os.ErrNotExist) {
 		return err
 	}
 	if err := os.Symlink("pts/ptmx", ptmx); err != nil {

libcontainer/specconv/spec_linux_test.go

@@ -1,6 +1,7 @@
 package specconv
 
 import (
+	"errors"
 	"os"
 	"strings"
 	"testing"
@@ -609,7 +610,7 @@ func TestDupNamespaces(t *testing.T) {
 }
 
 func TestUserNamespaceMappingAndPath(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires userns.")
 	}
 
@@ -643,7 +644,7 @@ func TestUserNamespaceMappingAndPath(t *testing.T) {
 }
 
 func TestNonZeroEUIDCompatibleSpecconvValidate(t *testing.T) {
-	if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
+	if _, err := os.Stat("/proc/self/ns/user"); errors.Is(err, os.ErrNotExist) {
 		t.Skip("Test requires userns.")
 	}
 

libcontainer/state_linux.go

@@ -1,6 +1,7 @@
 package libcontainer
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -213,7 +214,7 @@ func (r *restoredState) transition(s containerState) error {
 
 func (r *restoredState) destroy() error {
 	if _, err := os.Stat(filepath.Join(r.c.stateDir, "checkpoint")); err != nil {
-		if !os.IsNotExist(err) {
+		if !errors.Is(err, os.ErrNotExist) {
 			return err
 		}
 	}

spec.go

@@ -91,7 +91,7 @@ created by an unprivileged user.
 			if err == nil {
 				return fmt.Errorf("File %s exists. Remove it first", name)
 			}
-			if !os.IsNotExist(err) {
+			if !errors.Is(err, os.ErrNotExist) {
 				return err
 			}
 			return nil
@@ -117,7 +117,7 @@ created by an unprivileged user.
 func loadSpec(cPath string) (spec *specs.Spec, err error) {
 	cf, err := os.Open(cPath)
 	if err != nil {
-		if os.IsNotExist(err) {
+		if errors.Is(err, os.ErrNotExist) {
 			return nil, fmt.Errorf("JSON specification file %s not found", cPath)
 		}
 		return nil, err