Browse code

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>

Brian Goff authored on 2016/12/23 01:49:47
Showing 2 changed files
... ...
@@ -3,17 +3,13 @@ package store
3 3
 import (
4 4
 	"encoding/json"
5 5
 
6
+	"github.com/Sirupsen/logrus"
6 7
 	"github.com/boltdb/bolt"
7 8
 	"github.com/pkg/errors"
8 9
 )
9 10
 
10 11
 var volumeBucketName = []byte("volumes")
11 12
 
12
-type dbEntry struct {
13
-	Key   []byte
14
-	Value []byte
15
-}
16
-
17 13
 type volumeMetadata struct {
18 14
 	Name    string
19 15
 	Driver  string
... ...
@@ -67,12 +63,26 @@ func removeMeta(tx *bolt.Tx, name string) error {
67 67
 	return errors.Wrap(b.Delete([]byte(name)), "error removing volume metadata")
68 68
 }
69 69
 
70
-func listEntries(tx *bolt.Tx) []*dbEntry {
71
-	var entries []*dbEntry
70
+// listMeta is used during restore to get the list of volume metadata
71
+// from the on-disk database.
72
+// Any errors that occur are only logged.
73
+func listMeta(tx *bolt.Tx) []volumeMetadata {
74
+	var ls []volumeMetadata
72 75
 	b := tx.Bucket(volumeBucketName)
73 76
 	b.ForEach(func(k, v []byte) error {
74
-		entries = append(entries, &dbEntry{k, v})
77
+		if len(v) == 0 {
78
+			// don't try to unmarshal an empty value
79
+			return nil
80
+		}
81
+
82
+		var m volumeMetadata
83
+		if err := json.Unmarshal(v, &m); err != nil {
84
+			// Just log the error
85
+			logrus.Errorf("Error while reading volume metadata for volume %q: %v", string(k), err)
86
+			return nil
87
+		}
88
+		ls = append(ls, m)
75 89
 		return nil
76 90
 	})
77
-	return entries
91
+	return ls
78 92
 }
... ...
@@ -1,7 +1,6 @@
1 1
 package store
2 2
 
3 3
 import (
4
-	"encoding/json"
5 4
 	"sync"
6 5
 
7 6
 	"github.com/Sirupsen/logrus"
... ...
@@ -17,45 +16,38 @@ import (
17 17
 // It does not probe the available drivers to find anything that may have been added
18 18
 // out of band.
19 19
 func (s *VolumeStore) restore() {
20
-	var entries []*dbEntry
20
+	var ls []volumeMetadata
21 21
 	s.db.View(func(tx *bolt.Tx) error {
22
-		entries = listEntries(tx)
22
+		ls = listMeta(tx)
23 23
 		return nil
24 24
 	})
25 25
 
26
-	chRemove := make(chan []byte, len(entries))
26
+	chRemove := make(chan *volumeMetadata, len(ls))
27 27
 	var wg sync.WaitGroup
28
-	for _, entry := range entries {
28
+	for _, meta := range ls {
29 29
 		wg.Add(1)
30 30
 		// this is potentially a very slow operation, so do it in a goroutine
31
-		go func(entry *dbEntry) {
31
+		go func(meta volumeMetadata) {
32 32
 			defer wg.Done()
33
-			var meta volumeMetadata
34
-			if len(entry.Value) != 0 {
35
-				if err := json.Unmarshal(entry.Value, &meta); err != nil {
36
-					logrus.Errorf("Error while reading volume metadata for volume %q: %v", string(entry.Key), err)
37
-					// don't return here, we can try with `getVolume` below
38
-				}
39
-			}
40 33
 
41 34
 			var v volume.Volume
42 35
 			var err error
43 36
 			if meta.Driver != "" {
44
-				v, err = lookupVolume(meta.Driver, string(entry.Key))
37
+				v, err = lookupVolume(meta.Driver, meta.Name)
45 38
 				if err != nil && err != errNoSuchVolume {
46
-					logrus.WithError(err).WithField("driver", meta.Driver).WithField("volume", string(entry.Key)).Warn("Error restoring volume")
39
+					logrus.WithError(err).WithField("driver", meta.Driver).WithField("volume", meta.Name).Warn("Error restoring volume")
47 40
 					return
48 41
 				}
49 42
 				if v == nil {
50 43
 					// doesn't exist in the driver, remove it from the db
51
-					chRemove <- entry.Key
44
+					chRemove <- &meta
52 45
 					return
53 46
 				}
54 47
 			} else {
55
-				v, err = s.getVolume(string(entry.Key))
48
+				v, err = s.getVolume(meta.Name)
56 49
 				if err != nil {
57 50
 					if err == errNoSuchVolume {
58
-						chRemove <- entry.Key
51
+						chRemove <- &meta
59 52
 					}
60 53
 					return
61 54
 				}
... ...
@@ -75,15 +67,15 @@ func (s *VolumeStore) restore() {
75 75
 			s.labels[v.Name()] = meta.Labels
76 76
 			s.names[v.Name()] = v
77 77
 			s.globalLock.Unlock()
78
-		}(entry)
78
+		}(meta)
79 79
 	}
80 80
 
81 81
 	wg.Wait()
82 82
 	close(chRemove)
83 83
 	s.db.Update(func(tx *bolt.Tx) error {
84
-		for k := range chRemove {
85
-			if err := removeMeta(tx, string(k)); err != nil {
86
-				logrus.Warnf("Error removing stale entry from volume db: %v", err)
84
+		for meta := range chRemove {
85
+			if err := removeMeta(tx, meta.Name); err != nil {
86
+				logrus.WithField("volume", meta.Name).Warnf("Error removing stale entry from volume db: %v", err)
87 87
 			}
88 88
 		}
89 89
 		return nil