diff --git a/.ko.yaml b/.ko.yaml index 8d18fbcec5..e49180e73f 100644 --- a/.ko.yaml +++ b/.ko.yaml @@ -23,7 +23,3 @@ baseImageOverrides: # atelet needs glibc to run the node's kubelet credential provider, which # some platforms ship dynamically linked (e.g. AKS, GKE Ubuntu). github.com/agent-substrate/substrate/cmd/atelet: gcr.io/distroless/base-debian13:latest@sha256:389cad21f73e4c37b94ffe5b13736d5a92bd5bd3c6c6b38c2be1c881e14ba2bd - # ateom-microvm needs glibc (for the fetched cloud-hypervisor binary) and mount/umount - # (to bind the image into the virtiofsd shared dir) — both in debian:13-slim but - # not in the distroless static default. - github.com/agent-substrate/substrate/cmd/ateom-microvm: debian:13-slim@sha256:d7e12182ce18b85b93007c1dedf31f2d29e01ccf3182cc4017c709b6259bc132 diff --git a/cmd/ateom-microvm/internal/ch/merge.go b/cmd/ateom-microvm/internal/ch/merge.go index d25c9e045b..c88775f6a1 100644 --- a/cmd/ateom-microvm/internal/ch/merge.go +++ b/cmd/ateom-microvm/internal/ch/merge.go @@ -17,15 +17,14 @@ package ch import ( + "bytes" "context" "errors" "fmt" "io" "os" - "os/exec" "syscall" - "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/reaper" "golang.org/x/sys/unix" ) @@ -51,8 +50,8 @@ func MergeSparseOverlay(ctx context.Context, baseFile, deltaFile, outFile string // outFile := sparse copy of baseFile (preserves holes so it stays sparse). tmp := outFile + ".merge.tmp" _ = os.Remove(tmp) - if o, err := reaper.RunCombined(exec.CommandContext(ctx, "cp", "--sparse=always", baseFile, tmp)); err != nil { - return fmt.Errorf("cp base->tmp: %w: %s", err, o) + if err := copySparseFile(ctx, baseFile, tmp); err != nil { + return fmt.Errorf("sparse copy base->tmp: %w", err) } d, err := os.Open(deltaFile) @@ -238,3 +237,106 @@ func copySparseRegions(src, dst *os.File) (copied int64, err error) { } return copied, nil } + +// copySparseFile creates dstPath as a sparse copy of srcPath, like +// cp --sparse=always: holes in srcPath stay holes, and so does any all-zero +// block inside its data regions. +func copySparseFile(ctx context.Context, srcPath, dstPath string) error { + s, err := os.Open(srcPath) + if err != nil { + return err + } + defer s.Close() + si, err := s.Stat() + if err != nil { + return err + } + d, err := os.OpenFile(dstPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600) + if err != nil { + return err + } + defer d.Close() + // A fresh file of the full size reads as zeros, so a skipped block needs no write. + if err := d.Truncate(si.Size()); err != nil { + return err + } + if err := copyNonZeroRegions(ctx, s, d, si.Size()); err != nil { + return err + } + return d.Close() +} + +// sparseBlock is the granularity at which copyNonZeroRegions leaves zeros as holes. +const sparseBlock = 4096 + +// copyNonZeroRegions writes every block of src's data regions that is not all +// zeros to the same offset in dst. dst must read as zeros wherever nothing is +// written, as a freshly truncated file does, so unlike copySparseRegions this +// cannot overlay onto existing data: a skipped zero block would leave the old +// bytes in place. +func copyNonZeroRegions(ctx context.Context, src, dst *os.File, size int64) error { + sfd := int(src.Fd()) + buf := make([]byte, 1<<20) + for off := int64(0); off < size; { + ds, err := unix.Seek(sfd, off, unix.SEEK_DATA) + if errors.Is(err, unix.ENXIO) { + return nil // no more data + } + if err != nil { + return fmt.Errorf("SEEK_DATA: %w", err) + } + de, err := unix.Seek(sfd, ds, unix.SEEK_HOLE) + if err != nil { + return fmt.Errorf("SEEK_HOLE: %w", err) + } + for pos := ds; pos < de; { + if err := ctx.Err(); err != nil { + return err + } + n := min(int64(len(buf)), de-pos) + if _, err := src.ReadAt(buf[:n], pos); err != nil { + return fmt.Errorf("reading data region: %w", err) + } + if err := writeNonZero(dst, buf[:n], pos); err != nil { + return err + } + pos += n + } + off = de + } + return nil +} + +// writeNonZero writes p to dst at off, skipping all-zero blocks and coalescing +// the rest into as few writes as possible. +func writeNonZero(dst *os.File, p []byte, off int64) error { + start := -1 // start of the current run of non-zero blocks + for i := 0; i < len(p); i += sparseBlock { + if !allZero(p[i:min(i+sparseBlock, len(p))]) { + if start < 0 { + start = i + } + continue + } + if start >= 0 { + if _, err := dst.WriteAt(p[start:i], off+int64(start)); err != nil { + return err + } + start = -1 + } + } + if start >= 0 { + if _, err := dst.WriteAt(p[start:], off+int64(start)); err != nil { + return err + } + } + return nil +} + +// zeroBlock is what allZero compares against: bytes.Equal is vectorized, so it +// beats checking byte by byte. +var zeroBlock = make([]byte, sparseBlock) + +func allZero(b []byte) bool { + return bytes.Equal(b, zeroBlock[:len(b)]) +} diff --git a/cmd/ateom-microvm/internal/ch/merge_test.go b/cmd/ateom-microvm/internal/ch/merge_test.go index d9285ec75a..ddb3620889 100644 --- a/cmd/ateom-microvm/internal/ch/merge_test.go +++ b/cmd/ateom-microvm/internal/ch/merge_test.go @@ -248,3 +248,34 @@ func TestMergeDeltaIntoBaseSizeMismatch(t *testing.T) { t.Errorf("base should be intact after a refused merge: %v", err) } } + +// Zero runs inside a data region come out as holes, as with cp --sparse=always. +func TestCopySparseFileLeavesZeroBlocksAsHoles(t *testing.T) { + dir := t.TempDir() + // 1 MiB of written (allocated) zeros with one non-zero page in the middle. + data := make([]byte, 1<<20) + copy(data[512<<10:], bytes.Repeat([]byte{0xab}, 4096)) + src := filepath.Join(dir, "src") + if err := os.WriteFile(src, data, 0o600); err != nil { + t.Fatal(err) + } + dst := filepath.Join(dir, "dst") + if err := copySparseFile(context.Background(), src, dst); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(dst) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, data) { + t.Fatal("copy differs from source") + } + var st syscall.Stat_t + if err := syscall.Stat(dst, &st); err != nil { + t.Fatal(err) + } + // st.Blocks counts 512-byte units; allow slack for the filesystem's block size. + if allocated := st.Blocks * 512; allocated > 64<<10 { + t.Errorf("dst allocates %d bytes, want the zero runs left as holes", allocated) + } +} diff --git a/cmd/ateom-microvm/internal/kata/overlay_linux.go b/cmd/ateom-microvm/internal/kata/overlay_linux.go index 5c6b9c86a8..66e97d0634 100644 --- a/cmd/ateom-microvm/internal/kata/overlay_linux.go +++ b/cmd/ateom-microvm/internal/kata/overlay_linux.go @@ -35,14 +35,13 @@ import ( "os" "os/exec" "path/filepath" - "strings" "syscall" "time" - "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/reaper" "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/third_party/kata/agentpb" "github.com/agent-substrate/substrate/internal/ocispec" specs "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" ) const ( @@ -178,6 +177,31 @@ func waitForSocket(ctx context.Context, path string, timeout time.Duration) erro } } +// Unmount drops a mount at dst, falling back to lazy unmount if busy. +func Unmount(dst string) { + if err := unix.Unmount(dst, 0); err != nil { + if errors.Is(err, syscall.EINVAL) || errors.Is(err, syscall.ENOENT) { + return + } + _ = unix.Unmount(dst, unix.MNT_DETACH) + } +} + +// RemountReadOnly remounts a bind mount at dst as read-only, preserving existing +// per-mount flags (nosuid, nodev, noexec, etc.) like libmount's remount,bind,ro. +func RemountReadOnly(dst string) error { + var st unix.Statfs_t + if err := unix.Statfs(dst, &st); err != nil { + return fmt.Errorf("statfs %q: %w", dst, err) + } + flags := unix.MS_BIND | unix.MS_REMOUNT | unix.MS_RDONLY | + int(st.Flags&(unix.ST_NOSUID|unix.ST_NODEV|unix.ST_NOEXEC|unix.ST_NOATIME|unix.ST_NODIRATIME)) + if err := unix.Mount("", dst, "", uintptr(flags), ""); err != nil { + return err + } + return nil +} + // StageImageVolume bind-mounts one composed image volume read-only at // /volumes/ under SharedDir(id), so virtiofsd exposes it to the // guest. @@ -191,11 +215,8 @@ func StageImageVolume(ctx context.Context, src, id, cid, volumeName string) erro // Read-only is this volume type's contract (the image is someone else's, // mounted for its contents); the other subtree consumers stay writable. dst := SharedVolumeDir(id, cid, volumeName) - ro := exec.CommandContext(ctx, "mount", "-o", "remount,bind,ro", dst) - var roErr strings.Builder - ro.Stderr = &roErr - if err := reaper.Run(ro); err != nil { - return fmt.Errorf("remounting image volume %q read-only: %w (%s)", dst, err, strings.TrimSpace(roErr.String())) + if err := RemountReadOnly(dst); err != nil { + return fmt.Errorf("remounting image volume %q read-only: %w", dst, err) } return nil } @@ -217,9 +238,7 @@ func StageMergedRootfs(ctx context.Context, bundleRootfs, upperBase, restoreID, dst := filepath.Join(SharedDir(restoreID), cid, "rootfs") upper, work := UpperWorkDirs(upperBase, cid) // Drop any stale mount first (lazy if busy), then ensure clean mountpoints. - if err := reaper.Run(exec.Command("umount", dst)); err != nil { - _ = reaper.Run(exec.Command("umount", "-l", dst)) - } + Unmount(dst) // The workdir is scratch: wipe it so a volatile mount is never refused by a // dirty marker left behind by the previous activation. if err := os.RemoveAll(work); err != nil { @@ -247,11 +266,8 @@ func StageMergedRootfs(ctx context.Context, bundleRootfs, upperBase, restoreID, // a previous volatile mount, hence the wipe above. opts := "lowerdir=" + bundleRootfs + ",upperdir=" + upper + ",workdir=" + work + ",metacopy=off,index=off,volatile" - cmd := exec.CommandContext(ctx, "mount", "-t", "overlay", "overlay", "-o", opts, dst) - var stderr strings.Builder - cmd.Stderr = &stderr - if err := reaper.Run(cmd); err != nil { - return fmt.Errorf("mounting merged rootfs overlay at %q: %w (%s)", dst, err, strings.TrimSpace(stderr.String())) + if err := unix.Mount("overlay", dst, "overlay", 0, opts); err != nil { + return fmt.Errorf("mounting merged rootfs overlay at %q: %w", dst, err) } // Ensure the standard OCI mountpoints exist even for minimal images: the container // mounts /proc,/sys,/dev over them, and find-paths re-opens the tree by path on @@ -288,9 +304,7 @@ func ensureOCIMountpoints(rootfs string) error { // CleanupSandboxState's sweep catches stragglers on the next boot. func UnmountMergedRootfs(restoreID, cid string) { dst := filepath.Join(SharedDir(restoreID), cid, "rootfs") - if err := reaper.Run(exec.Command("umount", dst)); err != nil { - _ = reaper.Run(exec.Command("umount", "-l", dst)) - } + Unmount(dst) } // BindIntoShare bind-mounts a host directory at SharedDir(id)/, so the @@ -319,20 +333,15 @@ func BindIntoShare(ctx context.Context, src, id, rel string) error { } dst := filepath.Join(SharedDir(id), rel) // Drop any stale bind first (lazy if busy), then ensure a clean mountpoint. - if err := reaper.Run(exec.Command("umount", dst)); err != nil { - _ = reaper.Run(exec.Command("umount", "-l", dst)) - } + Unmount(dst) if err := os.MkdirAll(dst, 0o755); err != nil { return fmt.Errorf("creating share subdir %q: %w", dst, err) } // --rbind, not --bind: a source that is itself a mount (a composed image // volume) comes along either way, but only rbind carries mounts nested // beneath it; for the plain-directory sources it is the same operation. - cmd := exec.CommandContext(ctx, "mount", "--rbind", src, dst) - var stderr strings.Builder - cmd.Stderr = &stderr - if err := reaper.Run(cmd); err != nil { - return fmt.Errorf("bind-mounting %q into the shared tree at %q: %w (%s)", src, dst, err, strings.TrimSpace(stderr.String())) + if err := unix.Mount(src, dst, "", unix.MS_BIND|unix.MS_REC, ""); err != nil { + return fmt.Errorf("bind-mounting %q into the shared tree at %q: %w", src, dst, err) } return nil } @@ -350,17 +359,12 @@ func ReconstructSharedDirFromImage(ctx context.Context, bundleRootfs, restoreID, dst := filepath.Join(SharedDir(restoreID), cid, "rootfs") // Drop any stale bind first (lazy if busy), then ensure a clean mountpoint. Not // RemoveAll: that would chase a live bind into bundleRootfs. - if err := reaper.Run(exec.Command("umount", dst)); err != nil { - _ = reaper.Run(exec.Command("umount", "-l", dst)) - } + Unmount(dst) if err := os.MkdirAll(dst, 0o755); err != nil { return fmt.Errorf("creating shared dir %q: %w", dst, err) } - cmd := exec.CommandContext(ctx, "mount", "--bind", bundleRootfs, dst) - var stderr strings.Builder - cmd.Stderr = &stderr - if err := reaper.Run(cmd); err != nil { - return fmt.Errorf("bind-mounting image rootfs %q -> %q: %w (%s)", bundleRootfs, dst, err, strings.TrimSpace(stderr.String())) + if err := unix.Mount(bundleRootfs, dst, "", unix.MS_BIND, ""); err != nil { + return fmt.Errorf("bind-mounting image rootfs %q -> %q: %w", bundleRootfs, dst, err) } // Ensure the standard OCI mountpoints exist even for minimal images: the container // mounts /proc,/sys,/dev over them, and find-paths re-opens the lower by path on @@ -370,11 +374,8 @@ func ReconstructSharedDirFromImage(ctx context.Context, bundleRootfs, restoreID, } // Remount read-only: the lower is immutable, so all writes go to the overlay upper // and it stays byte-identical across reconstructions (required by find-paths migration). - ro := exec.CommandContext(ctx, "mount", "-o", "remount,bind,ro", dst) - var roErr strings.Builder - ro.Stderr = &roErr - if err := reaper.Run(ro); err != nil { - return fmt.Errorf("remounting overlay lower read-only %q: %w (%s)", dst, err, strings.TrimSpace(roErr.String())) + if err := RemountReadOnly(dst); err != nil { + return fmt.Errorf("remounting overlay lower read-only %q: %w", dst, err) } return nil } diff --git a/cmd/ateom-microvm/internal/kata/overlay_linux_test.go b/cmd/ateom-microvm/internal/kata/overlay_linux_test.go index b7c435b329..f49e5a8cb8 100644 --- a/cmd/ateom-microvm/internal/kata/overlay_linux_test.go +++ b/cmd/ateom-microvm/internal/kata/overlay_linux_test.go @@ -17,11 +17,16 @@ package kata import ( + "errors" "os" "path/filepath" "slices" "strings" + "syscall" "testing" + + "github.com/agent-substrate/substrate/internal/roottest" + "golang.org/x/sys/unix" ) // The container rootfs is untrusted (the image below, the guest's own snapshot @@ -100,3 +105,52 @@ func TestVirtiofsdArgs(t *testing.T) { t.Errorf("args %v do not set --migration-on-error guest-error", args) } } + +func TestRemountReadOnly_NonExistent(t *testing.T) { + dst := filepath.Join(t.TempDir(), "nonexistent") + err := RemountReadOnly(dst) + if err == nil { + t.Fatalf("RemountReadOnly(%q) = nil, want error", dst) + } + if !strings.Contains(err.Error(), "statfs") { + t.Errorf("RemountReadOnly(%q) error = %v, want error from statfs", dst, err) + } +} + +func TestUnmount_NonExistent(t *testing.T) { + dst := filepath.Join(t.TempDir(), "nonexistent") + // Unmount must handle non-existent destinations cleanly without panicking. + Unmount(dst) +} + +func TestRemountReadOnlyAndUnmount(t *testing.T) { + roottest.Require(t, "bind mounts") + dir := t.TempDir() + src := filepath.Join(dir, "src") + dst := filepath.Join(dir, "dst") + if err := os.Mkdir(src, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(dst, 0o755); err != nil { + t.Fatal(err) + } + if err := unix.Mount(src, dst, "", unix.MS_BIND, ""); err != nil { + t.Fatalf("Mount(%q, %q, BIND) = %v", src, dst, err) + } + defer Unmount(dst) + + testFile := filepath.Join(dst, "test.txt") + if err := os.WriteFile(testFile, []byte("write test"), 0o644); err != nil { + t.Fatalf("WriteFile before remount = %v", err) + } + + if err := RemountReadOnly(dst); err != nil { + t.Fatalf("RemountReadOnly(%q) = %v", dst, err) + } + + if err := os.WriteFile(testFile, []byte("should fail"), 0o644); !errors.Is(err, syscall.EROFS) { + t.Fatalf("WriteFile after remount = %v, want EROFS", err) + } + + Unmount(dst) +} diff --git a/cmd/ateom-microvm/systeminfo.go b/cmd/ateom-microvm/systeminfo.go index 57e485e40f..d237b9e262 100644 --- a/cmd/ateom-microvm/systeminfo.go +++ b/cmd/ateom-microvm/systeminfo.go @@ -45,14 +45,12 @@ import ( "context" "fmt" "os" - "os/exec" "path/filepath" - "strings" "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/kata" - "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/reaper" "github.com/agent-substrate/substrate/internal/ocispec" "github.com/agent-substrate/substrate/internal/proto/ateompb" + "golang.org/x/sys/unix" ) // hasSystemInfoVolumes reports whether any container mounts a system-info @@ -76,23 +74,15 @@ func (s *AteomService) stageSystemInfoVolumes(ctx context.Context, actorUID, src } dst := filepath.Join(kata.SharedDir(actorUID), ocispec.ShareSystemInfo) // Drop any stale mount first (lazy if busy), then ensure clean mountpoint. - if err := reaper.Run(exec.Command("umount", dst)); err != nil { - _ = reaper.Run(exec.Command("umount", "-l", dst)) - } + kata.Unmount(dst) if err := os.MkdirAll(dst, 0o755); err != nil { return fmt.Errorf("creating %q: %w", dst, err) } - cmd := exec.CommandContext(ctx, "mount", "--bind", src, dst) - var stderr strings.Builder - cmd.Stderr = &stderr - if err := reaper.Run(cmd); err != nil { - return fmt.Errorf("bind-mounting system-info volumes at %q: %w (%s)", dst, err, strings.TrimSpace(stderr.String())) + if err := unix.Mount(src, dst, "", unix.MS_BIND, ""); err != nil { + return fmt.Errorf("bind-mounting system-info volumes at %q: %w", dst, err) } - ro := exec.CommandContext(ctx, "mount", "-o", "remount,bind,ro", dst) - var roErr strings.Builder - ro.Stderr = &roErr - if err := reaper.Run(ro); err != nil { - return fmt.Errorf("remounting system-info volumes read-only %q: %w (%s)", dst, err, strings.TrimSpace(roErr.String())) + if err := kata.RemountReadOnly(dst); err != nil { + return fmt.Errorf("remounting system-info volumes read-only %q: %w", dst, err) } return nil }