Browse code

Safer file io for configuration files

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

Tonis Tiigi authored on 2016/04/21 09:08:47
Showing 7 changed files
... ...
@@ -23,6 +23,7 @@ import (
23 23
 	"github.com/docker/docker/image"
24 24
 	"github.com/docker/docker/layer"
25 25
 	"github.com/docker/docker/pkg/idtools"
26
+	"github.com/docker/docker/pkg/ioutils"
26 27
 	"github.com/docker/docker/pkg/promise"
27 28
 	"github.com/docker/docker/pkg/signal"
28 29
 	"github.com/docker/docker/pkg/symlink"
... ...
@@ -131,7 +132,7 @@ func (container *Container) ToDisk() error {
131 131
 		return err
132 132
 	}
133 133
 
134
-	jsonSource, err := os.Create(pth)
134
+	jsonSource, err := ioutils.NewAtomicFileWriter(pth, 0666)
135 135
 	if err != nil {
136 136
 		return err
137 137
 	}
... ...
@@ -191,7 +192,7 @@ func (container *Container) WriteHostConfig() error {
191 191
 		return err
192 192
 	}
193 193
 
194
-	f, err := os.Create(pth)
194
+	f, err := ioutils.NewAtomicFileWriter(pth, 0666)
195 195
 	if err != nil {
196 196
 		return err
197 197
 	}
... ...
@@ -5,6 +5,8 @@ import (
5 5
 	"os"
6 6
 	"path/filepath"
7 7
 	"sync"
8
+
9
+	"github.com/docker/docker/pkg/ioutils"
8 10
 )
9 11
 
10 12
 // Store implements a K/V store for mapping distribution-related IDs
... ...
@@ -56,14 +58,10 @@ func (store *FSMetadataStore) Set(namespace, key string, value []byte) error {
56 56
 	defer store.Unlock()
57 57
 
58 58
 	path := store.path(namespace, key)
59
-	tempFilePath := path + ".tmp"
60 59
 	if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
61 60
 		return err
62 61
 	}
63
-	if err := ioutil.WriteFile(tempFilePath, value, 0644); err != nil {
64
-		return err
65
-	}
66
-	return os.Rename(tempFilePath, path)
62
+	return ioutils.AtomicWriteFile(path, value, 0644)
67 63
 }
68 64
 
69 65
 // Delete removes data indexed by namespace and key. The data file named after
... ...
@@ -9,6 +9,7 @@ import (
9 9
 
10 10
 	"github.com/Sirupsen/logrus"
11 11
 	"github.com/docker/distribution/digest"
12
+	"github.com/docker/docker/pkg/ioutils"
12 13
 )
13 14
 
14 15
 // IDWalkFunc is function called by StoreBackend.Walk
... ...
@@ -118,12 +119,7 @@ func (s *fs) Set(data []byte) (ID, error) {
118 118
 	}
119 119
 
120 120
 	id := ID(digest.FromBytes(data))
121
-	filePath := s.contentFile(id)
122
-	tempFilePath := s.contentFile(id) + ".tmp"
123
-	if err := ioutil.WriteFile(tempFilePath, data, 0600); err != nil {
124
-		return "", err
125
-	}
126
-	if err := os.Rename(tempFilePath, filePath); err != nil {
121
+	if err := ioutils.AtomicWriteFile(s.contentFile(id), data, 0600); err != nil {
127 122
 		return "", err
128 123
 	}
129 124
 
... ...
@@ -156,12 +152,7 @@ func (s *fs) SetMetadata(id ID, key string, data []byte) error {
156 156
 	if err := os.MkdirAll(baseDir, 0700); err != nil {
157 157
 		return err
158 158
 	}
159
-	filePath := filepath.Join(s.metadataDir(id), key)
160
-	tempFilePath := filePath + ".tmp"
161
-	if err := ioutil.WriteFile(tempFilePath, data, 0600); err != nil {
162
-		return err
163
-	}
164
-	return os.Rename(tempFilePath, filePath)
159
+	return ioutils.AtomicWriteFile(filepath.Join(s.metadataDir(id), key), data, 0600)
165 160
 }
166 161
 
167 162
 // GetMetadata returns metadata for a given ID.
... ...
@@ -19,6 +19,7 @@ import (
19 19
 	"github.com/docker/docker/image"
20 20
 	imagev1 "github.com/docker/docker/image/v1"
21 21
 	"github.com/docker/docker/layer"
22
+	"github.com/docker/docker/pkg/ioutils"
22 23
 	"github.com/docker/docker/reference"
23 24
 )
24 25
 
... ...
@@ -160,12 +161,7 @@ func calculateLayerChecksum(graphDir, id string, ls checksumCalculator) error {
160 160
 		return err
161 161
 	}
162 162
 
163
-	tmpFile := filepath.Join(graphDir, id, migrationDiffIDFileName+".tmp")
164
-	if err := ioutil.WriteFile(tmpFile, []byte(diffID), 0600); err != nil {
165
-		return err
166
-	}
167
-
168
-	if err := os.Rename(tmpFile, filepath.Join(graphDir, id, migrationDiffIDFileName)); err != nil {
163
+	if err := ioutils.AtomicWriteFile(filepath.Join(graphDir, id, migrationDiffIDFileName), []byte(diffID), 0600); err != nil {
169 164
 		return err
170 165
 	}
171 166
 
172 167
new file mode 100644
... ...
@@ -0,0 +1,75 @@
0
+package ioutils
1
+
2
+import (
3
+	"io"
4
+	"io/ioutil"
5
+	"os"
6
+	"path/filepath"
7
+)
8
+
9
+// NewAtomicFileWriter returns WriteCloser so that writing to it writes to a
10
+// temporary file and closing it atomically changes the temporary file to
11
+// destination path. Writing and closing concurrently is not allowed.
12
+func NewAtomicFileWriter(filename string, perm os.FileMode) (io.WriteCloser, error) {
13
+	f, err := ioutil.TempFile(filepath.Dir(filename), ".tmp-"+filepath.Base(filename))
14
+	if err != nil {
15
+		return nil, err
16
+	}
17
+	abspath, err := filepath.Abs(filename)
18
+	if err != nil {
19
+		return nil, err
20
+	}
21
+	return &atomicFileWriter{
22
+		f:  f,
23
+		fn: abspath,
24
+	}, nil
25
+}
26
+
27
+// AtomicWriteFile atomically writes data to a file named by filename.
28
+func AtomicWriteFile(filename string, data []byte, perm os.FileMode) error {
29
+	f, err := NewAtomicFileWriter(filename, perm)
30
+	if err != nil {
31
+		return err
32
+	}
33
+	n, err := f.Write(data)
34
+	if err == nil && n < len(data) {
35
+		err = io.ErrShortWrite
36
+	}
37
+	if err1 := f.Close(); err == nil {
38
+		err = err1
39
+	}
40
+	return err
41
+}
42
+
43
+type atomicFileWriter struct {
44
+	f        *os.File
45
+	fn       string
46
+	writeErr error
47
+}
48
+
49
+func (w *atomicFileWriter) Write(dt []byte) (int, error) {
50
+	n, err := w.f.Write(dt)
51
+	if err != nil {
52
+		w.writeErr = err
53
+	}
54
+	return n, err
55
+}
56
+
57
+func (w *atomicFileWriter) Close() (retErr error) {
58
+	defer func() {
59
+		if retErr != nil {
60
+			os.Remove(w.f.Name())
61
+		}
62
+	}()
63
+	if err := w.f.Sync(); err != nil {
64
+		w.f.Close()
65
+		return err
66
+	}
67
+	if err := w.f.Close(); err != nil {
68
+		return err
69
+	}
70
+	if w.writeErr == nil {
71
+		return os.Rename(w.f.Name(), w.fn)
72
+	}
73
+	return nil
74
+}
0 75
new file mode 100644
... ...
@@ -0,0 +1,31 @@
0
+package ioutils
1
+
2
+import (
3
+	"bytes"
4
+	"io/ioutil"
5
+	"os"
6
+	"path/filepath"
7
+	"testing"
8
+)
9
+
10
+func TestAtomicWriteToFile(t *testing.T) {
11
+	tmpDir, err := ioutil.TempDir("", "atomic-writers-test")
12
+	if err != nil {
13
+		t.Fatalf("Error when creating temporary directory: %s", err)
14
+	}
15
+	defer os.RemoveAll(tmpDir)
16
+
17
+	expected := []byte("barbaz")
18
+	if err := AtomicWriteFile(filepath.Join(tmpDir, "foo"), expected, 0600); err != nil {
19
+		t.Fatalf("Error writing to file: %v", err)
20
+	}
21
+
22
+	actual, err := ioutil.ReadFile(filepath.Join(tmpDir, "foo"))
23
+	if err != nil {
24
+		t.Fatalf("Error reading from file: %v", err)
25
+	}
26
+
27
+	if bytes.Compare(actual, expected) != 0 {
28
+		t.Fatalf("Data mismatch, expected %q, got %q", expected, actual)
29
+	}
30
+}
... ...
@@ -4,7 +4,6 @@ import (
4 4
 	"encoding/json"
5 5
 	"errors"
6 6
 	"fmt"
7
-	"io/ioutil"
8 7
 	"os"
9 8
 	"path/filepath"
10 9
 	"sort"
... ...
@@ -12,6 +11,7 @@ import (
12 12
 
13 13
 	"github.com/docker/distribution/digest"
14 14
 	"github.com/docker/docker/image"
15
+	"github.com/docker/docker/pkg/ioutils"
15 16
 )
16 17
 
17 18
 var (
... ...
@@ -256,18 +256,7 @@ func (store *store) save() error {
256 256
 	if err != nil {
257 257
 		return err
258 258
 	}
259
-
260
-	tempFilePath := store.jsonPath + ".tmp"
261
-
262
-	if err := ioutil.WriteFile(tempFilePath, jsonData, 0600); err != nil {
263
-		return err
264
-	}
265
-
266
-	if err := os.Rename(tempFilePath, store.jsonPath); err != nil {
267
-		return err
268
-	}
269
-
270
-	return nil
259
+	return ioutils.AtomicWriteFile(store.jsonPath, jsonData, 0600)
271 260
 }
272 261
 
273 262
 func (store *store) reload() error {