From 4fffcf33dba8444e2cc99d6c41fe9459d230782d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 7 Aug 2025 18:35:08 +0200 Subject: [PATCH 1/3] libimage: add missing manifest lock Neither AnnotateInstance() or RemoveInstance() use the lock so we were still open to many races. Fixes: https://issues.redhat.com/browse/RHEL-21291 Signed-off-by: Paul Holzinger --- libimage/manifest_list.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/libimage/manifest_list.go b/libimage/manifest_list.go index d9140db7c..220219a98 100644 --- a/libimage/manifest_list.go +++ b/libimage/manifest_list.go @@ -669,6 +669,18 @@ func (m *ManifestList) AnnotateInstance(d digest.Digest, options *ManifestListAn return nil } + locker, err := manifests.LockerForImage(m.image.runtime.store, m.ID()) + if err != nil { + return err + } + locker.Lock() + defer locker.Unlock() + // Make sure to reload the image from the containers storage to fetch + // the latest data (e.g., new or delete digests). + if err := m.reload(); err != nil { + return err + } + if len(options.OS) > 0 { if err := m.list.SetOS(d, options.OS); err != nil { return err @@ -752,6 +764,18 @@ func (m *ManifestList) AnnotateInstance(d digest.Digest, options *ManifestListAn // RemoveInstance removes the instance specified by `d` from the manifest list. // Returns the new ID of the image. func (m *ManifestList) RemoveInstance(d digest.Digest) error { + locker, err := manifests.LockerForImage(m.image.runtime.store, m.ID()) + if err != nil { + return err + } + locker.Lock() + defer locker.Unlock() + // Make sure to reload the image from the containers storage to fetch + // the latest data (e.g., new or delete digests). + if err := m.reload(); err != nil { + return err + } + if err := m.list.Remove(d); err != nil { return err } From 7da5c7a2877948e8820dd81b38333f64b894d75b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 7 Aug 2025 18:47:07 +0200 Subject: [PATCH 2/3] libimage: dedup manifest reload() and saveAndReload() Share code as they do the same thing. Signed-off-by: Paul Holzinger --- libimage/manifest_list.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/libimage/manifest_list.go b/libimage/manifest_list.go index 220219a98..c93021be0 100644 --- a/libimage/manifest_list.go +++ b/libimage/manifest_list.go @@ -319,24 +319,16 @@ func (m *ManifestList) saveAndReload() error { if err != nil { return err } - - // Make sure to reload the image from the containers storage to fetch - // the latest data (e.g., new or delete digests). - if err := m.image.reload(); err != nil { - return err - } - image, list, err := m.image.runtime.lookupManifestList(newID) - if err != nil { - return err - } - m.image = image - m.list = list - return nil + return m.reloadID(newID) } // Reload the image and list instances from storage func (m *ManifestList) reload() error { listID := m.ID() + return m.reloadID(listID) +} + +func (m *ManifestList) reloadID(listID string) error { if err := m.image.reload(); err != nil { return err } From 18a0916593f7c40fe201e5ee02d150518301dd49 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 7 Aug 2025 18:50:45 +0200 Subject: [PATCH 3/3] libimage: remove extra image reload in manifest reload This does nothing it is just overhead. The lookupManifestList() call returns a new image which when then use and that also already calls image.reload() internally. Signed-off-by: Paul Holzinger --- libimage/manifest_list.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/libimage/manifest_list.go b/libimage/manifest_list.go index c93021be0..f7b2b05f0 100644 --- a/libimage/manifest_list.go +++ b/libimage/manifest_list.go @@ -329,9 +329,6 @@ func (m *ManifestList) reload() error { } func (m *ManifestList) reloadID(listID string) error { - if err := m.image.reload(); err != nil { - return err - } image, list, err := m.image.runtime.lookupManifestList(listID) if err != nil { return err