From 88dc04cb63f1ea933a7e730690d7410623d7764f Mon Sep 17 00:00:00 2001 From: Chaser Huang Date: Wed, 1 Apr 2026 16:01:42 -0400 Subject: [PATCH] overlay: fix incorrect diff folder permission with userns and fuse Overlay storage driver creates diff (upper) folder with mapped root IDs when userns is enabled. This is consistent with the behavior of the driver when using native overlayfs, where all lower layer FS will get ID mapped into the userns but upper layer is mounted as is. However when userns is used with fuse-overlayfs, the fuse driver performs id map over the entire FS. Creating diff dir with mapped IDs will cause the fuse layer to map it as nobody in the userns, causing the root folder unwritable in container Fixes cri-o/cri-o#9865 Signed-off-by: Chaser Huang --- storage/drivers/overlay/overlay.go | 36 +++++++++++++++++++----------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 5e38e9ab49..f18530b2d2 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -1535,16 +1535,32 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO return "", err } - // user namespace requires this to move a directory from lower to upper. - rootUID, rootGID, err := idtools.GetRootUIDGID(options.UidMaps, options.GidMaps) - if err != nil { - return "", err + if !d.SupportsShifting(options.UidMaps, options.GidMaps) || options.DisableShifting { + disableShifting = true } - rootIDs := idtools.IDPair{UID: rootUID, GID: rootGID} + + needsIDMapping := !disableShifting && len(options.UidMaps) > 0 && len(options.GidMaps) > 0 && d.options.mountProgram == "" + + // Owner IDs for upper directory + upperRootUID, upperRootGID := 0, 0 + // Use user namespace mapped root IDs here if and only if: + // a. Mount level ID shifting is completely disabled, container sees FS IDs as is, or + // b. Native ID mapping is used for ID shifting, where only lower layers will be mapped. + // Use of userns id here will allow data interchanges between lower and upper + // Note that exception is when fuse-overlayfs is used (mountProgram != "") + // where the entire FS is ID mapped together and there is no need to change ID here + if disableShifting || needsIDMapping { + rootUID, rootGID, err := idtools.GetRootUIDGID(options.UidMaps, options.GidMaps) + if err != nil { + return "", err + } + upperRootUID, upperRootGID = rootUID, rootGID + } + upperRootIDs := idtools.IDPair{UID: upperRootUID, GID: upperRootGID} mergedDir := d.getMergedDir(id, dir, inAdditionalStore) // Attempt to create the merged dir if it doesn't exist, but don't chown an already existing directory (it might be in an additional store) - if err := idtools.MkdirAllAndChownNew(mergedDir, 0o700, rootIDs); err != nil && !os.IsExist(err) { + if err := idtools.MkdirAllAndChownNew(mergedDir, 0o700, upperRootIDs); err != nil && !os.IsExist(err) { return "", err } @@ -1567,18 +1583,12 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO readWrite := !inAdditionalStore - if !d.SupportsShifting(options.UidMaps, options.GidMaps) || options.DisableShifting { - disableShifting = true - } - logLevel := logrus.WarnLevel if unshare.IsRootless() { logLevel = logrus.DebugLevel } optsList := options.Options - needsIDMapping := !disableShifting && len(options.UidMaps) > 0 && len(options.GidMaps) > 0 && d.options.mountProgram == "" - if len(optsList) == 0 { if d.options.mountOptions != "" { optsList = strings.Split(d.options.mountOptions, ",") @@ -1794,7 +1804,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO absLowers = append(absLowers, path.Join(dir, "empty")) } - if err := idtools.MkdirAllAndChown(diffDir, perms, rootIDs); err != nil { + if err := idtools.MkdirAllAndChown(diffDir, perms, upperRootIDs); err != nil { if !inAdditionalStore { return "", err }