From 24a4273cf96c34e90c5406146682e710d50a1121 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Fri, 28 Jul 2017 13:56:33 +0200 Subject: [PATCH 1/2] libcontainer: handle error cases Handle err return value of fmt.Scanf, os.Pipe and unix.ParseUnixRights. Found with honnef.co/go/tools/cmd/staticcheck Signed-off-by: Tobias Klauser --- libcontainer/container_linux.go | 6 ++++++ libcontainer/integration/exec_test.go | 6 ++++++ libcontainer/integration/execin_test.go | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 7a319ef8c..a21455da8 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -596,6 +596,9 @@ func (c *linuxContainer) checkCriuVersion(minVersion string) error { _, err := fmt.Sscanf(minVersion, "%d.%d.%d\n", &x, &y, &z) // 1.5.2 if err != nil { _, err = fmt.Sscanf(minVersion, "Version: %d.%d\n", &x, &y) // 1.6 + if err != nil { + return fmt.Errorf("Unable to parse the CRIU min version: %s", minVersion) + } } versionReq = x*10000 + y*100 + z @@ -1295,6 +1298,9 @@ func (c *linuxContainer) criuNotifications(resp *criurpc.CriuResp, process *Proc return err } fds, err := unix.ParseUnixRights(&scm[0]) + if err != nil { + return err + } master := os.NewFile(uintptr(fds[0]), "orphan-pts-master") defer master.Close() diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 49db94ac7..4fbd760fd 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -807,7 +807,13 @@ func TestPassExtraFiles(t *testing.T) { var stdout bytes.Buffer pipeout1, pipein1, err := os.Pipe() + if err != nil { + t.Fatal(err) + } pipeout2, pipein2, err := os.Pipe() + if err != nil { + t.Fatal(err) + } process := libcontainer.Process{ Cwd: "/", Args: []string{"sh", "-c", "cd /proc/$$/fd; echo -n *; echo -n 1 >3; echo -n 2 >4"}, diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 0f662bd3f..d8cb594e0 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -431,7 +431,13 @@ func TestExecinPassExtraFiles(t *testing.T) { var stdout bytes.Buffer pipeout1, pipein1, err := os.Pipe() + if err != nil { + t.Fatal(err) + } pipeout2, pipein2, err := os.Pipe() + if err != nil { + t.Fatal(err) + } inprocess := &libcontainer.Process{ Cwd: "/", Args: []string{"sh", "-c", "cd /proc/$$/fd; echo -n *; echo -n 1 >3; echo -n 2 >4"}, From e4e56cb6d8020232ac7fb79d0fb17307f8a409c3 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Fri, 28 Jul 2017 15:02:56 +0200 Subject: [PATCH 2/2] libcontainer: remove ineffective break statements go's switch statement doesn't need an explicit break. Remove it where that is the case and add a comment to indicate the purpose where the removal would lead to an empty case. Found with honnef.co/go/tools/cmd/staticcheck Signed-off-by: Tobias Klauser --- libcontainer/cgroups/systemd/apply_systemd.go | 2 -- libcontainer/container_linux.go | 8 +------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/libcontainer/cgroups/systemd/apply_systemd.go b/libcontainer/cgroups/systemd/apply_systemd.go index 456c57d97..b010b4b32 100644 --- a/libcontainer/cgroups/systemd/apply_systemd.go +++ b/libcontainer/cgroups/systemd/apply_systemd.go @@ -353,7 +353,6 @@ func joinCgroups(c *configs.Cgroup, pid int) error { switch name { case "name=systemd": // let systemd handle this - break case "cpuset": path, err := getSubsystemPath(c, name) if err != nil && !cgroups.IsNotFound(err) { @@ -363,7 +362,6 @@ func joinCgroups(c *configs.Cgroup, pid int) error { if err := s.ApplyDir(path, c, pid); err != nil { return err } - break default: _, err := join(c, name, pid) if err != nil { diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index a21455da8..612b17cd8 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -803,7 +803,6 @@ func (c *linuxContainer) Checkpoint(criuOpts *CriuOpts) error { switch m.Device { case "bind": c.addCriuDumpMount(req, m) - break case "cgroup": binds, err := getCgroupMounts(m) if err != nil { @@ -812,7 +811,6 @@ func (c *linuxContainer) Checkpoint(criuOpts *CriuOpts) error { for _, b := range binds { c.addCriuDumpMount(req, b) } - break } } @@ -865,9 +863,8 @@ func (c *linuxContainer) restoreNetwork(req *criurpc.CriuReq, criuOpts *CriuOpts veth.IfOut = proto.String(iface.HostInterfaceName) veth.IfIn = proto.String(iface.Name) req.Opts.Veths = append(req.Opts.Veths, veth) - break case "loopback": - break + // Do nothing } } for _, i := range criuOpts.VethPairs { @@ -957,7 +954,6 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error { switch m.Device { case "bind": c.addCriuRestoreMount(req, m) - break case "cgroup": binds, err := getCgroupMounts(m) if err != nil { @@ -966,7 +962,6 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error { for _, b := range binds { c.addCriuRestoreMount(req, b) } - break } } @@ -1156,7 +1151,6 @@ func (c *linuxContainer) criuSwrk(process *Process, req *criurpc.CriuReq, opts * case t == criurpc.CriuReqType_FEATURE_CHECK: logrus.Debugf("Feature check says: %s", resp) criuFeatures = resp.GetFeatures() - break case t == criurpc.CriuReqType_NOTIFY: if err := c.criuNotifications(resp, process, opts, extFds, oob[:oobn]); err != nil { return err