Skip to content

Commit 5aa9cde

Browse files
committed
store: get exclusive access to store with Diff/Changes
when NaiveDiff is used, the Diff/Changes operations can trigger the mount of the layer. Prevent that multiple processes step on each other and one of them performs an unmount while the other one is still accessing the mount. Closes: containers#2033 Signed-off-by: Giuseppe Scrivano <[email protected]>
1 parent 0af94a8 commit 5aa9cde

File tree

1 file changed

+56
-22
lines changed

1 file changed

+56
-22
lines changed

store.go

+56-22
Original file line numberDiff line numberDiff line change
@@ -2930,15 +2930,40 @@ func (s *store) Unmount(id string, force bool) (bool, error) {
29302930
}
29312931

29322932
func (s *store) Changes(from, to string) ([]archive.Change, error) {
2933-
if res, done, err := readAllLayerStores(s, func(store roLayerStore) ([]archive.Change, bool, error) {
2934-
if store.Exists(to) {
2935-
res, err := store.Changes(from, to)
2936-
return res, true, err
2937-
}
2938-
return nil, false, nil
2939-
}); done {
2933+
// NaiveDiff could cause mounts to happen without a lock, so be safe
2934+
// and treat the .Diff operation as a Mount.
2935+
// We need to make sure the home mount is present when the Mount is done, which happens by possibly reinitializing the graph driver
2936+
// in startUsingGraphDriver().
2937+
if err := s.startUsingGraphDriver(); err != nil {
2938+
return nil, err
2939+
}
2940+
defer s.stopUsingGraphDriver()
2941+
2942+
rlstore, lstores, err := s.bothLayerStoreKindsLocked()
2943+
if err != nil {
2944+
return nil, err
2945+
}
2946+
if err := rlstore.startWriting(); err != nil {
2947+
return nil, err
2948+
}
2949+
if rlstore.Exists(to) {
2950+
res, err := rlstore.Changes(from, to)
2951+
rlstore.stopWriting()
29402952
return res, err
29412953
}
2954+
rlstore.stopWriting()
2955+
2956+
for _, s := range lstores {
2957+
store := s
2958+
if err := store.startReading(); err != nil {
2959+
return nil, err
2960+
}
2961+
exists := store.Exists(to)
2962+
store.stopReading()
2963+
if exists {
2964+
return nil, fmt.Errorf("mounting read/only store images is not allowed: %w", ErrStoreIsReadOnly)
2965+
}
2966+
}
29422967
return nil, ErrLayerUnknown
29432968
}
29442969

@@ -2968,30 +2993,39 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro
29682993
}
29692994
defer s.stopUsingGraphDriver()
29702995

2971-
layerStores, err := s.allLayerStoresLocked()
2996+
rlstore, lstores, err := s.bothLayerStoreKindsLocked()
29722997
if err != nil {
29732998
return nil, err
29742999
}
29753000

2976-
for _, s := range layerStores {
3001+
if err := rlstore.startWriting(); err != nil {
3002+
return nil, err
3003+
}
3004+
if rlstore.Exists(to) {
3005+
rc, err := rlstore.Diff(from, to, options)
3006+
if rc != nil && err == nil {
3007+
wrapped := ioutils.NewReadCloserWrapper(rc, func() error {
3008+
err := rc.Close()
3009+
rlstore.stopWriting()
3010+
return err
3011+
})
3012+
return wrapped, nil
3013+
}
3014+
rlstore.stopWriting()
3015+
return rc, err
3016+
}
3017+
rlstore.stopWriting()
3018+
3019+
for _, s := range lstores {
29773020
store := s
29783021
if err := store.startReading(); err != nil {
29793022
return nil, err
29803023
}
2981-
if store.Exists(to) {
2982-
rc, err := store.Diff(from, to, options)
2983-
if rc != nil && err == nil {
2984-
wrapped := ioutils.NewReadCloserWrapper(rc, func() error {
2985-
err := rc.Close()
2986-
store.stopReading()
2987-
return err
2988-
})
2989-
return wrapped, nil
2990-
}
2991-
store.stopReading()
2992-
return rc, err
2993-
}
3024+
exists := store.Exists(to)
29943025
store.stopReading()
3026+
if exists {
3027+
return nil, fmt.Errorf("mounting read/only store images is not allowed: %w", ErrStoreIsReadOnly)
3028+
}
29953029
}
29963030
return nil, ErrLayerUnknown
29973031
}

0 commit comments

Comments
 (0)