From baf2e529b663767894f6220952f179bfa2c71e84 Mon Sep 17 00:00:00 2001 From: apostasie Date: Sat, 24 Aug 2024 16:49:03 -0700 Subject: [PATCH] Namestore hardening, workaround #3354 Signed-off-by: apostasie --- pkg/namestore/namestore.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/namestore/namestore.go b/pkg/namestore/namestore.go index 5243a8f958a..a6b57968c82 100644 --- a/pkg/namestore/namestore.go +++ b/pkg/namestore/namestore.go @@ -22,6 +22,8 @@ import ( "path/filepath" "strings" + "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/identifiers" "github.com/containerd/nerdctl/v2/pkg/lockutil" ) @@ -56,12 +58,21 @@ func (x *nameStore) Acquire(name, id string) error { } fn := func() error { fileName := filepath.Join(x.dir, name) - // If containerd was bounced, previously running containers that would get restarted will go again through - // onCreateRuntime (unlike in a "normal" stop/start flow). - // As such, we need to allow reacquiring by the same id - // See: https://github.com/containerd/nerdctl/issues/3354 - if b, err := os.ReadFile(fileName); err == nil && string(b) != id { - return fmt.Errorf("name %q is already used by ID %q", name, string(b)) + if b, err := os.ReadFile(fileName); err == nil { + if strings.TrimSpace(string(b)) == "" { + // currently acquired for an empty id - this obviously should never happen + // this is recoverable, and we are not hard erroring, but still indicative that something was wrong + // https://github.com/containerd/nerdctl/issues/3351 + log.L.Errorf("current name %q is reserved for a an empty id - please report this is as a bug", name) + } else if string(b) != id { + // if acquired by a different container, we error out here + return fmt.Errorf("name %q is already used by ID %q", name, string(b)) + } + // Otherwise, this is just re-acquiring after a restart + // For example, if containerd was bounced, previously running containers that would get restarted will go + // again through onCreateRuntime (unlike in a "normal" stop/start flow). + // As such, we are allowing reacquiring by the same id + // See: https://github.com/containerd/nerdctl/issues/3354 } return os.WriteFile(fileName, []byte(id), 0600) }