From 360c002932f715f2fe937f5116191dd811e385e5 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 17 Mar 2025 12:15:34 +0100 Subject: [PATCH] store: use canonical image mapping when mounting Ensure that a mapped layer with the canonical mapping is created if one doesn't already exist. Previously, any layer was used regardless of its mapping, which resulted in the mounted image having incorrect file ownership, depending on what mapping was found. Signed-off-by: Giuseppe Scrivano --- store.go | 132 +++++++++++++++++++++++++++++++++------------- tests/idmaps.bats | 12 ++++- 2 files changed, 107 insertions(+), 37 deletions(-) diff --git a/store.go b/store.go index 053c31c42..be1106761 100644 --- a/store.go +++ b/store.go @@ -2830,7 +2830,11 @@ func (s *store) Version() ([][2]string, error) { return [][2]string{}, nil } -func (s *store) mount(id string, options drivers.MountOpts) (string, error) { +func (s *store) MountImage(id string, mountOpts []string, mountLabel string) (string, error) { + if err := validateMountOptions(mountOpts); err != nil { + return "", err + } + // We need to make sure the home mount is present when the Mount is done, which happens by possibly reinitializing the graph driver // in startUsingGraphDriver(). if err := s.startUsingGraphDriver(); err != nil { @@ -2842,57 +2846,61 @@ func (s *store) mount(id string, options drivers.MountOpts) (string, error) { if err != nil { return "", err } - if options.UidMaps != nil || options.GidMaps != nil { - options.DisableShifting = !s.canUseShifting(options.UidMaps, options.GidMaps) - } + var imageHomeStore roImageStore - // function used to have a scope for rlstore.StopWriting() - tryMount := func() (string, error) { - if err := rlstore.startWriting(); err != nil { + if err := rlstore.startWriting(); err != nil { + return "", err + } + defer rlstore.stopWriting() + for _, s := range lstores { + if err := s.startReading(); err != nil { return "", err } - defer rlstore.stopWriting() - if rlstore.Exists(id) { - return rlstore.Mount(id, options) - } - return "", nil + defer s.stopReading() } - mountPoint, err := tryMount() - if mountPoint != "" || err != nil { - return mountPoint, err + if err := s.imageStore.startWriting(); err != nil { + return "", err } + defer s.imageStore.stopWriting() - // check if the layer is in a read-only store, and return a better error message - for _, store := range lstores { - if err := store.startReading(); err != nil { - return "", err - } - exists := store.Exists(id) - store.stopReading() - if exists { - return "", fmt.Errorf("mounting read/only store images is not allowed: %w", ErrStoreIsReadOnly) + cimage, err := s.imageStore.Get(id) + if err == nil { + imageHomeStore = s.imageStore + } else { + for _, s := range s.roImageStores { + if err := s.startReading(); err != nil { + return "", err + } + defer s.stopReading() + cimage, err = s.Get(id) + if err == nil { + imageHomeStore = s + break + } } } + if cimage == nil { + return "", fmt.Errorf("locating image with ID %q: %w", id, ErrImageUnknown) + } - return "", ErrLayerUnknown -} - -func (s *store) MountImage(id string, mountOpts []string, mountLabel string) (string, error) { - // Append ReadOnly option to mountOptions - img, err := s.Image(id) + idmappingsOpts := types.IDMappingOptions{ + HostUIDMapping: true, + HostGIDMapping: true, + } + ilayer, err := s.imageTopLayerForMapping(cimage, imageHomeStore, rlstore, lstores, idmappingsOpts) if err != nil { return "", err } - if err := validateMountOptions(mountOpts); err != nil { - return "", err + if len(ilayer.UIDMap) > 0 || len(ilayer.GIDMap) > 0 { + return "", fmt.Errorf("cannot create an image with canonical UID/GID mappings in a read-only store") } + options := drivers.MountOpts{ MountLabel: mountLabel, Options: append(mountOpts, "ro"), } - - return s.mount(img.TopLayer, options) + return rlstore.Mount(ilayer.ID, options) } func (s *store) Mount(id, mountLabel string) (string, error) { @@ -2914,7 +2922,43 @@ func (s *store) Mount(id, mountLabel string) (string, error) { } } } - return s.mount(id, options) + + // We need to make sure the home mount is present when the Mount is done, which happens by possibly reinitializing the graph driver + // in startUsingGraphDriver(). + if err := s.startUsingGraphDriver(); err != nil { + return "", err + } + defer s.stopUsingGraphDriver() + + rlstore, lstores, err := s.bothLayerStoreKindsLocked() + if err != nil { + return "", err + } + if options.UidMaps != nil || options.GidMaps != nil { + options.DisableShifting = !s.canUseShifting(options.UidMaps, options.GidMaps) + } + + if err := rlstore.startWriting(); err != nil { + return "", err + } + defer rlstore.stopWriting() + if rlstore.Exists(id) { + return rlstore.Mount(id, options) + } + + // check if the layer is in a read-only store, and return a better error message + for _, store := range lstores { + if err := store.startReading(); err != nil { + return "", err + } + exists := store.Exists(id) + store.stopReading() + if exists { + return "", fmt.Errorf("mounting read/only store images is not allowed: %w", ErrStoreIsReadOnly) + } + } + + return "", ErrLayerUnknown } func (s *store) Mounted(id string) (int, error) { @@ -2938,7 +2982,23 @@ func (s *store) UnmountImage(id string, force bool) (bool, error) { if err != nil { return false, err } - return s.Unmount(img.TopLayer, force) + + return writeToLayerStore(s, func(lstore rwLayerStore) (bool, error) { + for _, layerID := range img.MappedTopLayers { + l, err := lstore.Get(layerID) + if err != nil { + if err == ErrLayerUnknown { + continue + } + return false, err + } + // check if the layer with the canonical mapping is in the mapped top layers + if len(l.UIDMap) == 0 && len(l.GIDMap) == 0 { + return lstore.unmount(l.ID, force, false) + } + } + return lstore.unmount(img.TopLayer, force, false) + }) } func (s *store) Unmount(id string, force bool) (bool, error) { diff --git a/tests/idmaps.bats b/tests/idmaps.bats index 5b78538c0..a340360f7 100644 --- a/tests/idmaps.bats +++ b/tests/idmaps.bats @@ -290,7 +290,17 @@ load helpers done # Create new containers based on the layer. imagename=idmappedimage - storage create-image --name=$imagename $lowerlayer + run storage create-image --name=$imagename $lowerlayer + [ "$status" -eq 0 ] + + run storage --debug=false mount -r $imagename + [ "$status" -eq 0 ] + mountpoint="$output" + run stat -c %u:%g $mountpoint/file1 + # make sure the file shows the correct ownership after the image is mounted + [ "$output" == "0:0" ] + run storage umount $imagename + [ "$status" -eq 0 ] run storage --debug=false create-container --hostuidmap --hostgidmap $imagename [ "$status" -eq 0 ]