Browse code

container.BaseFS: check for nil before deref

Commit 7a7357dae1bccc ("LCOW: Implemented support for docker cp + build")
changed `container.BaseFS` from being a string (that could be empty but
can't lead to nil pointer dereference) to containerfs.ContainerFS,
which could be be `nil` and so nil dereference is at least theoretically
possible, which leads to panic (i.e. engine crashes).

Such a panic can be avoided by carefully analysing the source code in all
the places that dereference a variable, to make the variable can't be nil.
Practically, this analisys are impossible as code is constantly
evolving.

Still, we need to avoid panics and crashes. A good way to do so is to
explicitly check that a variable is non-nil, returning an error
otherwise. Even in case such a check looks absolutely redundant,
further changes to the code might make it useful, and having an
extra check is not a big price to pay to avoid a panic.

This commit adds such checks for all the places where it is not obvious
that container.BaseFS is not nil (which in this case means we do not
call daemon.Mount() a few lines earlier).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Kir Kolyshkin authored on 2018/03/14 11:45:21
Showing 4 changed files
... ...
@@ -6,6 +6,7 @@ import (
6 6
 	"github.com/docker/docker/api/types"
7 7
 	"github.com/docker/docker/pkg/archive"
8 8
 	"github.com/docker/docker/pkg/system"
9
+	"github.com/pkg/errors"
9 10
 )
10 11
 
11 12
 // ResolvePath resolves the given path in the container to a resource on the
... ...
@@ -13,6 +14,9 @@ import (
13 13
 // the absolute path to the resource relative to the container's rootfs, and
14 14
 // an error if the path points to outside the container's rootfs.
15 15
 func (container *Container) ResolvePath(path string) (resolvedPath, absPath string, err error) {
16
+	if container.BaseFS == nil {
17
+		return "", "", errors.New("ResolvePath: BaseFS of container " + container.ID + " is unexpectedly nil")
18
+	}
16 19
 	// Check if a drive letter supplied, it must be the system drive. No-op except on Windows
17 20
 	path, err = system.CheckSystemDriveAndRemoveDriveLetter(path, container.BaseFS)
18 21
 	if err != nil {
... ...
@@ -45,6 +49,9 @@ func (container *Container) ResolvePath(path string) (resolvedPath, absPath stri
45 45
 // resolved to a path on the host corresponding to the given absolute path
46 46
 // inside the container.
47 47
 func (container *Container) StatPath(resolvedPath, absPath string) (stat *types.ContainerPathStat, err error) {
48
+	if container.BaseFS == nil {
49
+		return nil, errors.New("StatPath: BaseFS of container " + container.ID + " is unexpectedly nil")
50
+	}
48 51
 	driver := container.BaseFS
49 52
 
50 53
 	lstat, err := driver.Lstat(resolvedPath)
... ...
@@ -311,6 +311,9 @@ func (container *Container) SetupWorkingDirectory(rootIDs idtools.IDPair) error
311 311
 //       symlinking to a different path) between using this method and using the
312 312
 //       path. See symlink.FollowSymlinkInScope for more details.
313 313
 func (container *Container) GetResourcePath(path string) (string, error) {
314
+	if container.BaseFS == nil {
315
+		return "", errors.New("GetResourcePath: BaseFS of container " + container.ID + " is unexpectedly nil")
316
+	}
314 317
 	// IMPORTANT - These are paths on the OS where the daemon is running, hence
315 318
 	// any filepath operations must be done in an OS agnostic way.
316 319
 	r, e := container.BaseFS.ResolveScopedPath(path, false)
... ...
@@ -705,6 +705,9 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c
705 705
 }
706 706
 
707 707
 func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container) error {
708
+	if c.BaseFS == nil {
709
+		return errors.New("populateCommonSpec: BaseFS of container " + c.ID + " is unexpectedly nil")
710
+	}
708 711
 	linkedEnv, err := daemon.setupLinkedContainers(c)
709 712
 	if err != nil {
710 713
 		return err
... ...
@@ -221,6 +221,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
221 221
 
222 222
 // Sets the Windows-specific fields of the OCI spec
223 223
 func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.Spec, isHyperV bool) error {
224
+	if c.BaseFS == nil {
225
+		return errors.New("createSpecWindowsFields: BaseFS of container " + c.ID + " is unexpectedly nil")
226
+	}
224 227
 	if len(s.Process.Cwd) == 0 {
225 228
 		// We default to C:\ to workaround the oddity of the case that the
226 229
 		// default directory for cmd running as LocalSystem (or