Skip to content

Commit 120642a

Browse files
committed
overlay: use private merged directory
use a private "merged" directory when mounting from an additional store. Operations like "Diff()" and "Changes()" cause an implicit mount when the naive differ is used. The issue was not observed earlier because native overlay can achieve these operations without using a mount. Since these mounts are performed read-only, and overlay supports multiple mounts using the same lowerdirs, use a private location for the "merged" directory. The location is owned by the current writeable store, that is locked for writing. Closes: containers#2033 Signed-off-by: Giuseppe Scrivano <[email protected]>
1 parent 5aa9cde commit 120642a

File tree

1 file changed

+30
-8
lines changed

1 file changed

+30
-8
lines changed

drivers/overlay/overlay.go

+30-8
Original file line numberDiff line numberDiff line change
@@ -848,14 +848,14 @@ func (d *Driver) Status() [][2]string {
848848
// Metadata returns meta data about the overlay driver such as
849849
// LowerDir, UpperDir, WorkDir and MergeDir used to store data.
850850
func (d *Driver) Metadata(id string) (map[string]string, error) {
851-
dir := d.dir(id)
851+
dir, _, inAdditionalStore := d.dir2(id, false)
852852
if err := fileutils.Exists(dir); err != nil {
853853
return nil, err
854854
}
855855

856856
metadata := map[string]string{
857857
"WorkDir": path.Join(dir, "work"),
858-
"MergedDir": path.Join(dir, "merged"),
858+
"MergedDir": d.getMergedDir(id, dir, inAdditionalStore),
859859
"UpperDir": path.Join(dir, "diff"),
860860
}
861861

@@ -1703,10 +1703,10 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
17031703
}
17041704
}
17051705

1706-
mergedDir := path.Join(dir, "merged")
1706+
mergedDir := d.getMergedDir(id, dir, inAdditionalStore)
17071707
// Attempt to create the merged dir only if it doesn't exist.
17081708
if err := fileutils.Exists(mergedDir); err != nil && os.IsNotExist(err) {
1709-
if err := idtools.MkdirAs(mergedDir, 0o700, rootUID, rootGID); err != nil && !os.IsExist(err) {
1709+
if err := idtools.MkdirAllAs(mergedDir, 0o700, rootUID, rootGID); err != nil && !os.IsExist(err) {
17101710
return "", err
17111711
}
17121712
}
@@ -1856,7 +1856,9 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
18561856
mountFunc = func(source string, target string, mType string, flags uintptr, label string) error {
18571857
return mountOverlayFrom(d.home, source, target, mType, flags, label)
18581858
}
1859-
mountTarget = path.Join(id, "merged")
1859+
if !inAdditionalStore {
1860+
mountTarget = path.Join(id, "merged")
1861+
}
18601862
}
18611863

18621864
// overlay has a check in place to prevent mounting the same file system twice
@@ -1875,13 +1877,25 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
18751877
return mergedDir, nil
18761878
}
18771879

1880+
// getMergedDir returns the directory path that should be used as the mount point for the overlayfs.
1881+
func (d *Driver) getMergedDir(id, dir string, inAdditionalStore bool) string {
1882+
// If the layer is in an additional store, the lock we might hold only a reading lock. To prevent
1883+
// races with other processes, use a private directory under the main store rundir, where we hold
1884+
// a read/write lock.
1885+
if inAdditionalStore {
1886+
return path.Join(d.runhome, id, "merged")
1887+
}
1888+
return path.Join(dir, "merged")
1889+
}
1890+
18781891
// Put unmounts the mount path created for the give id.
18791892
func (d *Driver) Put(id string) error {
18801893
dir, _, inAdditionalStore := d.dir2(id, false)
18811894
if err := fileutils.Exists(dir); err != nil {
18821895
return err
18831896
}
1884-
mountpoint := path.Join(dir, "merged")
1897+
mountpoint := d.getMergedDir(id, dir, inAdditionalStore)
1898+
18851899
if count := d.ctr.Decrement(mountpoint); count > 0 {
18861900
return nil
18871901
}
@@ -1938,7 +1952,15 @@ func (d *Driver) Put(id string) error {
19381952
}
19391953
}
19401954

1941-
if !inAdditionalStore {
1955+
if inAdditionalStore {
1956+
// check the base name for extra safety
1957+
if strings.HasPrefix(mountpoint, d.runhome) && filepath.Base(mountpoint) == "merged" {
1958+
err := os.RemoveAll(filepath.Dir(mountpoint))
1959+
if err != nil {
1960+
logrus.Warningf("Failed to remove mountpoint %s overlay: %s: %v", id, mountpoint, err)
1961+
}
1962+
}
1963+
} else {
19421964
uid, gid := int(0), int(0)
19431965
fi, err := os.Stat(mountpoint)
19441966
if err != nil {
@@ -1955,7 +1977,7 @@ func (d *Driver) Put(id string) error {
19551977
// rename(2) can be used on an empty directory, as it is the mountpoint after umount, and it retains
19561978
// its atomic semantic. In this way the "merged" directory is never removed.
19571979
if err := unix.Rename(tmpMountpoint, mountpoint); err != nil {
1958-
logrus.Debugf("Failed to replace mountpoint %s overlay: %s - %v", id, mountpoint, err)
1980+
logrus.Debugf("Failed to replace mountpoint %s overlay: %s: %v", id, mountpoint, err)
19591981
return fmt.Errorf("replacing mount point %q: %w", mountpoint, err)
19601982
}
19611983
}

0 commit comments

Comments
 (0)