From 4876a9047ebfd66294d88482a1b4b24634a632e6 Mon Sep 17 00:00:00 2001
From: Brian Goff <cpuguy83@gmail.com>
Date: Thu, 22 Dec 2016 11:49:47 -0500
Subject: [PATCH] Fix usage of boltdb in volume restore

bolt k/v pairs are only valid for the life of a transaction.
This means the memory that the k/v pair is referencing may be invalid if
it is accessed outside of the transaction.
This can potentially cause a panic.

For reference: https://godoc.org/github.com/boltdb/bolt#hdr-Caveats

To fix this issue, unmarshal the stored data into volume meta before
closing the transaction.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
---
 volume/store/db.go      | 28 +++++++++++++++++++---------
 volume/store/restore.go | 36 ++++++++++++++----------------------
 2 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/volume/store/db.go b/volume/store/db.go
index 0dc41509a4..c5fd1643f5 100644
--- a/volume/store/db.go
+++ b/volume/store/db.go
@@ -3,17 +3,13 @@ package store
 import (
 	"encoding/json"
 
+	"github.com/Sirupsen/logrus"
 	"github.com/boltdb/bolt"
 	"github.com/pkg/errors"
 )
 
 var volumeBucketName = []byte("volumes")
 
-type dbEntry struct {
-	Key   []byte
-	Value []byte
-}
-
 type volumeMetadata struct {
 	Name    string
 	Driver  string
@@ -67,12 +63,26 @@ func removeMeta(tx *bolt.Tx, name string) error {
 	return errors.Wrap(b.Delete([]byte(name)), "error removing volume metadata")
 }
 
-func listEntries(tx *bolt.Tx) []*dbEntry {
-	var entries []*dbEntry
+// listMeta is used during restore to get the list of volume metadata
+// from the on-disk database.
+// Any errors that occur are only logged.
+func listMeta(tx *bolt.Tx) []volumeMetadata {
+	var ls []volumeMetadata
 	b := tx.Bucket(volumeBucketName)
 	b.ForEach(func(k, v []byte) error {
-		entries = append(entries, &dbEntry{k, v})
+		if len(v) == 0 {
+			// don't try to unmarshal an empty value
+			return nil
+		}
+
+		var m volumeMetadata
+		if err := json.Unmarshal(v, &m); err != nil {
+			// Just log the error
+			logrus.Errorf("Error while reading volume metadata for volume %q: %v", string(k), err)
+			return nil
+		}
+		ls = append(ls, m)
 		return nil
 	})
-	return entries
+	return ls
 }
diff --git a/volume/store/restore.go b/volume/store/restore.go
index e20740ba30..c0c5b519bc 100644
--- a/volume/store/restore.go
+++ b/volume/store/restore.go
@@ -1,7 +1,6 @@
 package store
 
 import (
-	"encoding/json"
 	"sync"
 
 	"github.com/Sirupsen/logrus"
@@ -17,45 +16,38 @@ import (
 // It does not probe the available drivers to find anything that may have been added
 // out of band.
 func (s *VolumeStore) restore() {
-	var entries []*dbEntry
+	var ls []volumeMetadata
 	s.db.View(func(tx *bolt.Tx) error {
-		entries = listEntries(tx)
+		ls = listMeta(tx)
 		return nil
 	})
 
-	chRemove := make(chan []byte, len(entries))
+	chRemove := make(chan *volumeMetadata, len(ls))
 	var wg sync.WaitGroup
-	for _, entry := range entries {
+	for _, meta := range ls {
 		wg.Add(1)
 		// this is potentially a very slow operation, so do it in a goroutine
-		go func(entry *dbEntry) {
+		go func(meta volumeMetadata) {
 			defer wg.Done()
-			var meta volumeMetadata
-			if len(entry.Value) != 0 {
-				if err := json.Unmarshal(entry.Value, &meta); err != nil {
-					logrus.Errorf("Error while reading volume metadata for volume %q: %v", string(entry.Key), err)
-					// don't return here, we can try with `getVolume` below
-				}
-			}
 
 			var v volume.Volume
 			var err error
 			if meta.Driver != "" {
-				v, err = lookupVolume(meta.Driver, string(entry.Key))
+				v, err = lookupVolume(meta.Driver, meta.Name)
 				if err != nil && err != errNoSuchVolume {
-					logrus.WithError(err).WithField("driver", meta.Driver).WithField("volume", string(entry.Key)).Warn("Error restoring volume")
+					logrus.WithError(err).WithField("driver", meta.Driver).WithField("volume", meta.Name).Warn("Error restoring volume")
 					return
 				}
 				if v == nil {
 					// doesn't exist in the driver, remove it from the db
-					chRemove <- entry.Key
+					chRemove <- &meta
 					return
 				}
 			} else {
-				v, err = s.getVolume(string(entry.Key))
+				v, err = s.getVolume(meta.Name)
 				if err != nil {
 					if err == errNoSuchVolume {
-						chRemove <- entry.Key
+						chRemove <- &meta
 					}
 					return
 				}
@@ -75,15 +67,15 @@ func (s *VolumeStore) restore() {
 			s.labels[v.Name()] = meta.Labels
 			s.names[v.Name()] = v
 			s.globalLock.Unlock()
-		}(entry)
+		}(meta)
 	}
 
 	wg.Wait()
 	close(chRemove)
 	s.db.Update(func(tx *bolt.Tx) error {
-		for k := range chRemove {
-			if err := removeMeta(tx, string(k)); err != nil {
-				logrus.Warnf("Error removing stale entry from volume db: %v", err)
+		for meta := range chRemove {
+			if err := removeMeta(tx, meta.Name); err != nil {
+				logrus.WithField("volume", meta.Name).Warnf("Error removing stale entry from volume db: %v", err)
 			}
 		}
 		return nil