Skip to content

Commit 5fbbad3

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 a9357dd commit 5fbbad3

File tree

1 file changed

+58
-22
lines changed

1 file changed

+58
-22
lines changed

store.go

+58-22
Original file line numberDiff line numberDiff line change
@@ -2930,15 +2930,41 @@ 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", ErrLayerUnknown)
2965+
2966+
}
2967+
}
29422968
return nil, ErrLayerUnknown
29432969
}
29442970

@@ -2968,30 +2994,40 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro
29682994
}
29692995
defer s.stopUsingGraphDriver()
29702996

2971-
layerStores, err := s.allLayerStoresLocked()
2997+
rlstore, lstores, err := s.bothLayerStoreKindsLocked()
29722998
if err != nil {
29732999
return nil, err
29743000
}
29753001

2976-
for _, s := range layerStores {
3002+
if err := rlstore.startWriting(); err != nil {
3003+
return nil, err
3004+
}
3005+
if rlstore.Exists(to) {
3006+
rc, err := rlstore.Diff(from, to, options)
3007+
if rc != nil && err == nil {
3008+
wrapped := ioutils.NewReadCloserWrapper(rc, func() error {
3009+
err := rc.Close()
3010+
rlstore.stopWriting()
3011+
return err
3012+
})
3013+
return wrapped, nil
3014+
}
3015+
rlstore.stopWriting()
3016+
return rc, err
3017+
}
3018+
rlstore.stopWriting()
3019+
3020+
for _, s := range lstores {
29773021
store := s
29783022
if err := store.startReading(); err != nil {
29793023
return nil, err
29803024
}
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-
}
3025+
exists := store.Exists(to)
29943026
store.stopReading()
3027+
if exists {
3028+
return nil, fmt.Errorf("mounting read/only store images is not allowed: %w", ErrLayerUnknown)
3029+
3030+
}
29953031
}
29963032
return nil, ErrLayerUnknown
29973033
}

0 commit comments

Comments
 (0)