From f7c47e3f6c2354544723151adb5f7997e1c5b01b Mon Sep 17 00:00:00 2001 From: nileshpatil6 Date: Mon, 31 Aug 2026 21:44:19 +0530 Subject: [PATCH] cmd: fix limitedRead returning nil error when Stat fails In containerd-shim-runhcs-v1, limitedRead checked the Stat result inside an if statement, so the err in the final return referred to the outer err from os.Open, which is always nil at that point. errors.Wrapf returns nil for a nil error, so a Stat failure returned ([]byte{}, nil) and the delete handler silently skipped the shim panic log without logging a warning. Restructure the function the way the containerd-shim-lcow-v2 copy already does, and switch both copies from f.Read to io.ReadFull with the read count sliced off, so a short read cannot hand back NUL padding that was never in the file. Add the same limitedRead unit tests the lcow shim has. Signed-off-by: nileshpatil6 --- cmd/containerd-shim-lcow-v2/manager.go | 6 +-- cmd/containerd-shim-runhcs-v1/delete.go | 24 ++++++----- cmd/containerd-shim-runhcs-v1/delete_test.go | 44 ++++++++++++++++++++ 3 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 cmd/containerd-shim-runhcs-v1/delete_test.go diff --git a/cmd/containerd-shim-lcow-v2/manager.go b/cmd/containerd-shim-lcow-v2/manager.go index 6ab22af109..46c0d9a5d4 100644 --- a/cmd/containerd-shim-lcow-v2/manager.go +++ b/cmd/containerd-shim-lcow-v2/manager.go @@ -260,11 +260,11 @@ func limitedRead(filePath string, readLimitBytes int64) ([]byte, error) { readLimitBytes = fi.Size() } buf := make([]byte, readLimitBytes) - _, err = f.Read(buf) - if err != nil { + n, err := io.ReadFull(f, buf) + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) { return []byte{}, fmt.Errorf("read file %s: %w", filePath, err) } - return buf, nil + return buf[:n], nil } // Info returns runtime information about this shim including its name, version, diff --git a/cmd/containerd-shim-runhcs-v1/delete.go b/cmd/containerd-shim-runhcs-v1/delete.go index c1eb3375fa..83356576f4 100644 --- a/cmd/containerd-shim-runhcs-v1/delete.go +++ b/cmd/containerd-shim-runhcs-v1/delete.go @@ -5,6 +5,7 @@ package main import ( "context" "fmt" + "io" "os" "path/filepath" "time" @@ -30,18 +31,19 @@ func limitedRead(filePath string, readLimitBytes int64) ([]byte, error) { return nil, errors.Wrapf(err, "limited read failed to open file: %s", filePath) } defer f.Close() - if fi, err := f.Stat(); err == nil { - if fi.Size() < readLimitBytes { - readLimitBytes = fi.Size() - } - buf := make([]byte, readLimitBytes) - _, err := f.Read(buf) - if err != nil { - return []byte{}, errors.Wrapf(err, "limited read failed during file read: %s", filePath) - } - return buf, nil + fi, err := f.Stat() + if err != nil { + return []byte{}, errors.Wrapf(err, "limited read failed during file stat: %s", filePath) + } + if fi.Size() < readLimitBytes { + readLimitBytes = fi.Size() + } + buf := make([]byte, readLimitBytes) + n, err := io.ReadFull(f, buf) + if err != nil && err != io.ErrUnexpectedEOF { + return []byte{}, errors.Wrapf(err, "limited read failed during file read: %s", filePath) } - return []byte{}, errors.Wrapf(err, "limited read failed during file stat: %s", filePath) + return buf[:n], nil } var deleteCommand = cli.Command{ diff --git a/cmd/containerd-shim-runhcs-v1/delete_test.go b/cmd/containerd-shim-runhcs-v1/delete_test.go new file mode 100644 index 0000000000..3de0f53d53 --- /dev/null +++ b/cmd/containerd-shim-runhcs-v1/delete_test.go @@ -0,0 +1,44 @@ +//go:build windows + +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// TestLimitedRead verifies that limitedRead enforces the byte limit when the +// file is larger than the limit and reads the full content when the file is +// smaller than the limit. +func TestLimitedRead(t *testing.T) { + filePath := filepath.Join(t.TempDir(), "panic.log") + if err := os.WriteFile(filePath, []byte("hello"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + buf, err := limitedRead(filePath, 2) + if err != nil { + t.Fatalf("limitedRead: %v", err) + } + if string(buf) != "he" { + t.Fatalf("expected 'he', got %q", string(buf)) + } + + buf, err = limitedRead(filePath, 10) + if err != nil { + t.Fatalf("limitedRead: %v", err) + } + if string(buf) != "hello" { + t.Fatalf("expected 'hello', got %q", string(buf)) + } +} + +// TestLimitedReadMissingFile verifies that limitedRead returns an error when +// the target file does not exist. +func TestLimitedReadMissingFile(t *testing.T) { + _, err := limitedRead(filepath.Join(t.TempDir(), "missing.log"), 10) + if err == nil { + t.Fatalf("expected error for missing file") + } +}