From 195893d38160c0893e326b8674e05ef6714aeaa4 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Wed, 7 Feb 2018 15:25:50 -0800
Subject: [PATCH] c.RWLayer: check for nil before use

Since commit e9b9e4ace294230c6b8eb has landed, there is a chance that
container.RWLayer is nil (due to some half-removed container). Let's
check the pointer before use to avoid any potential nil pointer
dereferences, resulting in a daemon crash.

Note that even without the abovementioned commit, it's better to perform
an extra check (even it's totally redundant) rather than to have a
possibility of a daemon crash. In other words, better be safe than
sorry.

[v2: add a test case for daemon.getInspectData]
[v3: add a check for container.Dead and a special error for the case]

Fixes: e9b9e4ace294230c6b8eb
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 daemon/changes.go      |  3 +++
 daemon/daemon.go       |  6 ++++++
 daemon/inspect.go      | 17 ++++++++++++++---
 daemon/inspect_test.go | 33 +++++++++++++++++++++++++++++++++
 daemon/oci_windows.go  |  4 ++++
 5 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 daemon/inspect_test.go

diff --git a/daemon/changes.go b/daemon/changes.go
index 306790e888..70b3f6b943 100644
--- a/daemon/changes.go
+++ b/daemon/changes.go
@@ -22,6 +22,9 @@ func (daemon *Daemon) ContainerChanges(name string) ([]archive.Change, error) {
 
 	container.Lock()
 	defer container.Unlock()
+	if container.RWLayer == nil {
+		return nil, errors.New("RWLayer of container " + name + " is unexpectedly nil")
+	}
 	c, err := container.RWLayer.Changes()
 	if err != nil {
 		return nil, err
diff --git a/daemon/daemon.go b/daemon/daemon.go
index 982cdf543d..ab28a56ebe 100644
--- a/daemon/daemon.go
+++ b/daemon/daemon.go
@@ -1051,6 +1051,9 @@ func (daemon *Daemon) Shutdown() error {
 // Mount sets container.BaseFS
 // (is it not set coming in? why is it unset?)
 func (daemon *Daemon) Mount(container *container.Container) error {
+	if container.RWLayer == nil {
+		return errors.New("RWLayer of container " + container.ID + " is unexpectedly nil")
+	}
 	dir, err := container.RWLayer.Mount(container.GetMountLabel())
 	if err != nil {
 		return err
@@ -1073,6 +1076,9 @@ func (daemon *Daemon) Mount(container *container.Container) error {
 
 // Unmount unsets the container base filesystem
 func (daemon *Daemon) Unmount(container *container.Container) error {
+	if container.RWLayer == nil {
+		return errors.New("RWLayer of container " + container.ID + " is unexpectedly nil")
+	}
 	if err := container.RWLayer.Unmount(); err != nil {
 		logrus.Errorf("Error unmounting container %s: %s", container.ID, err)
 		return err
diff --git a/daemon/inspect.go b/daemon/inspect.go
index c38cee5675..164e1aa2ae 100644
--- a/daemon/inspect.go
+++ b/daemon/inspect.go
@@ -1,6 +1,7 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
@@ -184,14 +185,24 @@ func (daemon *Daemon) getInspectData(container *container.Container) (*types.Con
 
 	contJSONBase.GraphDriver.Name = container.Driver
 
+	if container.RWLayer == nil {
+		if container.Dead {
+			return contJSONBase, nil
+		}
+		return nil, errdefs.System(errors.New("RWLayer of container " + container.ID + " is unexpectedly nil"))
+	}
+
 	graphDriverData, err := container.RWLayer.Metadata()
 	// If container is marked as Dead, the container's graphdriver metadata
 	// could have been removed, it will cause error if we try to get the metadata,
 	// we can ignore the error if the container is dead.
-	if err != nil && !container.Dead {
-		return nil, errdefs.System(err)
+	if err != nil {
+		if !container.Dead {
+			return nil, errdefs.System(err)
+		}
+	} else {
+		contJSONBase.GraphDriver.Data = graphDriverData
 	}
-	contJSONBase.GraphDriver.Data = graphDriverData
 
 	return contJSONBase, nil
 }
diff --git a/daemon/inspect_test.go b/daemon/inspect_test.go
new file mode 100644
index 0000000000..c10cc56796
--- /dev/null
+++ b/daemon/inspect_test.go
@@ -0,0 +1,33 @@
+package daemon // import "github.com/docker/docker/daemon"
+
+import (
+	"testing"
+
+	containertypes "github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/container"
+	"github.com/docker/docker/daemon/config"
+	"github.com/docker/docker/daemon/exec"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetInspectData(t *testing.T) {
+	c := &container.Container{
+		ID:           "inspect-me",
+		HostConfig:   &containertypes.HostConfig{},
+		State:        container.NewState(),
+		ExecCommands: exec.NewStore(),
+	}
+
+	d := &Daemon{
+		linkIndex:   newLinkIndex(),
+		configStore: &config.Config{},
+	}
+
+	_, err := d.getInspectData(c)
+	assert.Error(t, err)
+
+	c.Dead = true
+	_, err = d.getInspectData(c)
+	assert.NoError(t, err)
+}
diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go
index 74440531d7..47b1301eee 100644
--- a/daemon/oci_windows.go
+++ b/daemon/oci_windows.go
@@ -1,6 +1,7 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
@@ -156,6 +157,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
 		// Reverse order, expecting parent most first
 		s.Windows.LayerFolders = append([]string{layerPath}, s.Windows.LayerFolders...)
 	}
+	if c.RWLayer == nil {
+		return nil, errors.New("RWLayer of container " + c.ID + " is unexpectedly nil")
+	}
 	m, err := c.RWLayer.Metadata()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get layer metadata - %s", err)