Преглед на файлове

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>
Kir Kolyshkin преди 7 години
родител
ревизия
195893d381
променени са 5 файла, в които са добавени 60 реда и са изтрити 3 реда
  1. 3 0
      daemon/changes.go
  2. 6 0
      daemon/daemon.go
  3. 14 3
      daemon/inspect.go
  4. 33 0
      daemon/inspect_test.go
  5. 4 0
      daemon/oci_windows.go

+ 3 - 0
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

+ 6 - 0
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

+ 14 - 3
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
 }

+ 33 - 0
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)
+}

+ 4 - 0
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)