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>
(cherry picked from commit 195893d38160c0893e326b8674e05ef6714aeaa4)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
diff --git a/daemon/changes.go b/daemon/changes.go
index fc8cd27..8b163b8 100644
--- a/daemon/changes.go
+++ b/daemon/changes.go
@@ -22,6 +22,9 @@
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 dd8c100..3727d95 100644
--- a/daemon/daemon.go
+++ b/daemon/daemon.go
@@ -1056,6 +1056,9 @@
// 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
@@ -1078,6 +1081,9 @@
// 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 20cfa6c..4e5fc72 100644
--- a/daemon/inspect.go
+++ b/daemon/inspect.go
@@ -1,6 +1,7 @@
package daemon
import (
+ "errors"
"fmt"
"time"
@@ -183,14 +184,24 @@
contJSONBase.GraphDriver.Name = container.Driver
+ if container.RWLayer == nil {
+ if container.Dead {
+ return contJSONBase, nil
+ }
+ return nil, systemError{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, systemError{err}
+ if err != nil {
+ if !container.Dead {
+ return nil, systemError{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 0000000..c10cc56
--- /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 c7c94f3..8647b3e 100644
--- a/daemon/oci_windows.go
+++ b/daemon/oci_windows.go
@@ -1,6 +1,7 @@
package daemon
import (
+ "errors"
"fmt"
"io/ioutil"
"path/filepath"
@@ -145,6 +146,9 @@
// 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)