From b45fbd43b8f120389ae978e9f856ac07f60d0663 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 1 Feb 2021 19:33:47 +0100 Subject: [PATCH] errcheck: libcontainer Signed-off-by: Sebastiaan van Stijn --- libcontainer/container_linux.go | 4 ++-- libcontainer/factory_linux.go | 6 +++++- libcontainer/factory_linux_test.go | 11 +++++++---- libcontainer/process_linux.go | 30 ++++++++++++++--------------- libcontainer/rootfs_linux.go | 16 +++++++-------- libcontainer/setns_init_linux.go | 4 ++-- libcontainer/standard_init_linux.go | 8 ++++---- libcontainer/state_linux.go | 2 +- 8 files changed, 44 insertions(+), 37 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 849bf4a61..5b65f8896 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -1352,7 +1352,7 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error { if err != nil { return err } - defer unix.Unmount(root, unix.MNT_DETACH) + defer unix.Unmount(root, unix.MNT_DETACH) //nolint: errcheck t := criurpc.CriuReqType_RESTORE req := &criurpc.CriuReq{ Type: &t, @@ -1665,7 +1665,7 @@ func (c *linuxContainer) criuSwrk(process *Process, req *criurpc.CriuReq, opts * break } - criuClientCon.CloseWrite() + _ = criuClientCon.CloseWrite() // cmd.Wait() waits cmd.goroutines which are used for proxying file descriptors. // Here we want to wait only the CRIU process. criuProcessState, err = criuProcess.Wait() diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 972b0f2b5..b3beba81e 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -196,7 +196,11 @@ func New(root string, options ...func(*LinuxFactory) error) (Factory, error) { Validator: validate.New(), CriuPath: "criu", } - Cgroupfs(l) + + if err := Cgroupfs(l); err != nil { + return nil, err + } + for _, opt := range options { if opt == nil { continue diff --git a/libcontainer/factory_linux_test.go b/libcontainer/factory_linux_test.go index 665a8a7c4..0b88c3d32 100644 --- a/libcontainer/factory_linux_test.go +++ b/libcontainer/factory_linux_test.go @@ -122,7 +122,10 @@ func TestFactoryNewTmpfs(t *testing.T) { if m.Source != "tmpfs" { t.Fatalf("Source of root: %s, expected %s", m.Source, "tmpfs") } - unix.Unmount(root, unix.MNT_DETACH) + err = unix.Unmount(root, unix.MNT_DETACH) + if err != nil { + t.Error("failed to unmount root:", err) + } } func TestFactoryLoadNotExists(t *testing.T) { @@ -130,7 +133,7 @@ func TestFactoryLoadNotExists(t *testing.T) { if rerr != nil { t.Fatal(rerr) } - defer os.RemoveAll(root) + defer os.RemoveAll(root) //nolint: errcheck factory, err := New(root, Cgroupfs) if err != nil { t.Fatal(err) @@ -153,7 +156,7 @@ func TestFactoryLoadContainer(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(root) + defer os.RemoveAll(root) //nolint: errcheck // setup default container config and state for mocking var ( id = "1" @@ -219,7 +222,7 @@ func marshal(path string, v interface{}) error { if err != nil { return err } - defer f.Close() + defer f.Close() //nolint: errcheck return utils.WriteJSON(f, v) } diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 053971b91..80f25e334 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -188,7 +188,7 @@ func (p *setnsProcess) start() (retErr error) { } // Must be done after Shutdown so the child will exit and we can wait for it. if ierr != nil { - p.wait() + _, _ = p.wait() return ierr } return nil @@ -201,16 +201,16 @@ func (p *setnsProcess) start() (retErr error) { func (p *setnsProcess) execSetns() error { status, err := p.cmd.Process.Wait() if err != nil { - p.cmd.Wait() + _ = p.cmd.Wait() return newSystemErrorWithCause(err, "waiting on setns process to finish") } if !status.Success() { - p.cmd.Wait() + _ = p.cmd.Wait() return newSystemError(&exec.ExitError{ProcessState: status}) } var pid *pid if err := json.NewDecoder(p.messageSockPair.parent).Decode(&pid); err != nil { - p.cmd.Wait() + _ = p.cmd.Wait() return newSystemErrorWithCause(err, "reading pid from init pipe") } @@ -292,7 +292,7 @@ func (p *initProcess) externalDescriptors() []string { func (p *initProcess) getChildPid() (int, error) { var pid pid if err := json.NewDecoder(p.messageSockPair.parent).Decode(&pid); err != nil { - p.cmd.Wait() + _ = p.cmd.Wait() return -1, err } @@ -309,11 +309,11 @@ func (p *initProcess) getChildPid() (int, error) { func (p *initProcess) waitForChildExit(childPid int) error { status, err := p.cmd.Process.Wait() if err != nil { - p.cmd.Wait() + _ = p.cmd.Wait() return err } if !status.Success() { - p.cmd.Wait() + _ = p.cmd.Wait() return &exec.ExitError{ProcessState: status} } @@ -327,12 +327,12 @@ func (p *initProcess) waitForChildExit(childPid int) error { } func (p *initProcess) start() (retErr error) { - defer p.messageSockPair.parent.Close() + defer p.messageSockPair.parent.Close() //nolint: errcheck err := p.cmd.Start() p.process.ops = p // close the write-side of the pipes (controlled by child) - p.messageSockPair.child.Close() - p.logFilePair.child.Close() + _ = p.messageSockPair.child.Close() + _ = p.logFilePair.child.Close() if err != nil { p.process.ops = nil return newSystemErrorWithCause(err, "starting init process command") @@ -371,9 +371,9 @@ func (p *initProcess) start() (retErr error) { logrus.WithError(err).Warn("unable to terminate initProcess") } - p.manager.Destroy() + _ = p.manager.Destroy() if p.intelRdtManager != nil { - p.intelRdtManager.Destroy() + _ = p.intelRdtManager.Destroy() } } }() @@ -553,7 +553,7 @@ func (p *initProcess) start() (retErr error) { // Must be done after Shutdown so the child will exit and we can wait for it. if ierr != nil { - p.wait() + _, _ = p.wait() return ierr } return nil @@ -563,7 +563,7 @@ func (p *initProcess) wait() (*os.ProcessState, error) { err := p.cmd.Wait() // we should kill all processes in cgroup when init is died if we use host PID namespace if p.sharePidns { - signalAllProcesses(p.manager, unix.SIGKILL) + _ = signalAllProcesses(p.manager, unix.SIGKILL) } return p.cmd.ProcessState, err } @@ -668,7 +668,7 @@ func (p *Process) InitializeIO(rootuid, rootgid int) (i *IO, err error) { defer func() { if err != nil { for _, fd := range fds { - unix.Close(int(fd)) + _ = unix.Close(int(fd)) } } }() diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index d9c5146dd..ff91b3dbc 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -196,9 +196,9 @@ func prepareTmp(topTmpDir string) (string, error) { return tmpdir, nil } -func cleanupTmp(tmpdir string) error { - unix.Unmount(tmpdir, 0) - return os.RemoveAll(tmpdir) +func cleanupTmp(tmpdir string) { + _ = unix.Unmount(tmpdir, 0) + _ = os.RemoveAll(tmpdir) } func mountCmd(cmd configs.Command) error { @@ -615,7 +615,7 @@ func reOpenDevNull() error { if err != nil { return fmt.Errorf("Failed to open /dev/null - %s", err) } - defer file.Close() + defer file.Close() //nolint: errcheck if err := unix.Fstat(int(file.Fd()), &devNullStat); err != nil { return err } @@ -661,7 +661,7 @@ func bindMountDeviceNode(rootfs, dest string, node *devices.Device) error { return err } if f != nil { - f.Close() + _ = f.Close() } return utils.WithProcfd(rootfs, dest, func(procfd string) error { return unix.Mount(node.Path, procfd, "bind", unix.MS_BIND, "") @@ -826,13 +826,13 @@ func pivotRoot(rootfs string) error { if err != nil { return err } - defer unix.Close(oldroot) + defer unix.Close(oldroot) //nolint: errcheck newroot, err := unix.Open(rootfs, unix.O_DIRECTORY|unix.O_RDONLY, 0) if err != nil { return err } - defer unix.Close(newroot) + defer unix.Close(newroot) //nolint: errcheck // Change to the new root so that the pivot_root actually acts on it. if err := unix.Fchdir(newroot); err != nil { @@ -956,7 +956,7 @@ func createIfNotExists(path string, isDir bool) error { if err != nil { return err } - f.Close() + _ = f.Close() } } return nil diff --git a/libcontainer/setns_init_linux.go b/libcontainer/setns_init_linux.go index 97987f1d0..2d7e5814e 100644 --- a/libcontainer/setns_init_linux.go +++ b/libcontainer/setns_init_linux.go @@ -37,7 +37,7 @@ func (l *linuxSetnsInit) Init() error { if err := selinux.SetKeyLabel(l.config.ProcessLabel); err != nil { return err } - defer selinux.SetKeyLabel("") + defer selinux.SetKeyLabel("") //nolint: errcheck // Do not inherit the parent's session keyring. if _, err := keys.JoinSessionKeyring(l.getSessionRingName()); err != nil { // Same justification as in standart_init_linux.go as to why we @@ -65,7 +65,7 @@ func (l *linuxSetnsInit) Init() error { if err := selinux.SetExecLabel(l.config.ProcessLabel); err != nil { return err } - defer selinux.SetExecLabel("") + defer selinux.SetExecLabel("") //nolint: errcheck // Without NoNewPrivileges seccomp is a privileged operation, so we need to // do this before dropping capabilities; otherwise do it as late as possible // just before execve so as few syscalls take place after it as possible. diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index d77022ad4..45c6d66cb 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -52,7 +52,7 @@ func (l *linuxStandardInit) Init() error { if err := selinux.SetKeyLabel(l.config.ProcessLabel); err != nil { return err } - defer selinux.SetKeyLabel("") + defer selinux.SetKeyLabel("") //nolint: errcheck ringname, keepperms, newperms := l.getSessionRingParams() // Do not inherit the parent's session keyring. @@ -151,7 +151,7 @@ func (l *linuxStandardInit) Init() error { if err := selinux.SetExecLabel(l.config.ProcessLabel); err != nil { return errors.Wrap(err, "set process label") } - defer selinux.SetExecLabel("") + defer selinux.SetExecLabel("") //nolint: errcheck // Without NoNewPrivileges seccomp is a privileged operation, so we need to // do this before dropping capabilities; otherwise do it as late as possible // just before execve so as few syscalls take place after it as possible. @@ -183,7 +183,7 @@ func (l *linuxStandardInit) Init() error { } // Close the pipe to signal that we have completed our init. logrus.Debugf("init: closing the pipe to signal completion") - l.pipe.Close() + _ = l.pipe.Close() // Close the log pipe fd so the parent's ForwardLogs can exit. if err := unix.Close(l.logFd); err != nil { @@ -207,7 +207,7 @@ func (l *linuxStandardInit) Init() error { // N.B. the core issue itself (passing dirfds to the host filesystem) has // since been resolved. // https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318 - unix.Close(l.fifoFd) + _ = unix.Close(l.fifoFd) // Set seccomp as close to execve as possible, so as few syscalls take // place afterward (reducing the amount of syscalls that users need to // enable in their seccomp profiles). diff --git a/libcontainer/state_linux.go b/libcontainer/state_linux.go index 02ff06ea9..43c040c85 100644 --- a/libcontainer/state_linux.go +++ b/libcontainer/state_linux.go @@ -157,7 +157,7 @@ func (i *createdState) transition(s containerState) error { } func (i *createdState) destroy() error { - i.c.initProcess.signal(unix.SIGKILL) + _ = i.c.initProcess.signal(unix.SIGKILL) return destroy(i.c) }