From 683e8065f7dfadc6afc75941cf55b79eef8f96ff Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 17 Jul 2024 17:49:08 +0200 Subject: [PATCH] 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: https://github.com/containers/storage/issues/2033 Signed-off-by: Giuseppe Scrivano --- drivers/overlay/overlay.go | 39 ++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index d94f99550d..4cbbd36fcb 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -848,14 +848,14 @@ func (d *Driver) Status() [][2]string { // Metadata returns meta data about the overlay driver such as // LowerDir, UpperDir, WorkDir and MergeDir used to store data. func (d *Driver) Metadata(id string) (map[string]string, error) { - dir := d.dir(id) + dir, _, inAdditionalStore := d.dir2(id, false) if err := fileutils.Exists(dir); err != nil { return nil, err } metadata := map[string]string{ "WorkDir": path.Join(dir, "work"), - "MergedDir": path.Join(dir, "merged"), + "MergedDir": d.getMergedDir(id, dir, inAdditionalStore), "UpperDir": path.Join(dir, "diff"), } @@ -1703,10 +1703,10 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO } } - mergedDir := path.Join(dir, "merged") + mergedDir := d.getMergedDir(id, dir, inAdditionalStore) // Attempt to create the merged dir only if it doesn't exist. if err := fileutils.Exists(mergedDir); err != nil && os.IsNotExist(err) { - if err := idtools.MkdirAs(mergedDir, 0o700, rootUID, rootGID); err != nil && !os.IsExist(err) { + if err := idtools.MkdirAllAs(mergedDir, 0o700, rootUID, rootGID); err != nil && !os.IsExist(err) { return "", err } } @@ -1856,7 +1856,9 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO mountFunc = func(source string, target string, mType string, flags uintptr, label string) error { return mountOverlayFrom(d.home, source, target, mType, flags, label) } - mountTarget = path.Join(id, "merged") + if !inAdditionalStore { + mountTarget = path.Join(id, "merged") + } } // overlay has a check in place to prevent mounting the same file system twice @@ -1875,13 +1877,26 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO return mergedDir, nil } +// getMergedDir returns the directory path that should be used as the mount point for the overlayfs. +func (d *Driver) getMergedDir(id, dir string, inAdditionalStore bool) string { + // If the layer is in an additional store, the lock we might hold only a reading lock. To prevent + // races with other processes, use a private directory under the main store rundir. At this point, the + // current process is holding an exclusive lock on the store, and since the rundir cannot be shared for + // different stores, it is safe to assume the current process has exclusive access to it. + if inAdditionalStore { + return path.Join(d.runhome, id, "merged") + } + return path.Join(dir, "merged") +} + // Put unmounts the mount path created for the give id. func (d *Driver) Put(id string) error { dir, _, inAdditionalStore := d.dir2(id, false) if err := fileutils.Exists(dir); err != nil { return err } - mountpoint := path.Join(dir, "merged") + mountpoint := d.getMergedDir(id, dir, inAdditionalStore) + if count := d.ctr.Decrement(mountpoint); count > 0 { return nil } @@ -1938,7 +1953,15 @@ func (d *Driver) Put(id string) error { } } - if !inAdditionalStore { + if inAdditionalStore { + // check the base name for extra safety + if strings.HasPrefix(mountpoint, d.runhome) && filepath.Base(mountpoint) == "merged" { + err := os.RemoveAll(filepath.Dir(mountpoint)) + if err != nil { + logrus.Warningf("Failed to remove mountpoint %s overlay: %s: %v", id, mountpoint, err) + } + } + } else { uid, gid := int(0), int(0) fi, err := os.Stat(mountpoint) if err != nil { @@ -1955,7 +1978,7 @@ func (d *Driver) Put(id string) error { // rename(2) can be used on an empty directory, as it is the mountpoint after umount, and it retains // its atomic semantic. In this way the "merged" directory is never removed. if err := unix.Rename(tmpMountpoint, mountpoint); err != nil { - logrus.Debugf("Failed to replace mountpoint %s overlay: %s - %v", id, mountpoint, err) + logrus.Debugf("Failed to replace mountpoint %s overlay: %s: %v", id, mountpoint, err) return fmt.Errorf("replacing mount point %q: %w", mountpoint, err) } }