fix(store): do not trust PID files left behind by a previous boot

`ReadPIDFile` decides whether a PID file belongs to a running process by signaling
the PID. Within a single boot this is fine, because a PID file of a process that
has already exited gets removed. Across a reboot it is not: the recorded PID may
have been reused by an unrelated process, and Lima then reports the instance as
`Broken`, either with "driver is running but host agent is not", or by trying to
connect to a stale `ha.sock`. `limactl start` fails, and a LaunchDaemon with
`KeepAlive` crash-loops it.

Record the boot session ID of the host next to the PID files of an instance, and
ignore a PID file that was written during a different boot. The ID is read from
the `kern.bootsessionuuid` sysctl on macOS and from
`/proc/sys/kernel/random/boot_id` on Linux; on platforms without such an
identifier the behavior is unchanged, as it is for instances that were started by
an older version of Lima and have no marker.

The marker is shared by the PID files of the directory, so writing a PID file
during a new boot removes all of them before the marker is refreshed: refreshing
it must never make a PID file of a previous boot look current. Readers check the
boot before they read the PID, and leave the removal to the next writer, so that
a reader can never remove a PID file that another process is writing at that very
moment. The PID file itself is renamed into place.

When the boot session ID cannot be determined the PID file is not written at all,
because a marker left over from a previous boot would otherwise void a PID file
that is current. Only the platforms without support for a boot session ID fall
back to the previous behavior.

The PID of a stale PID file is never signaled, because it may belong to an
unrelated process.

`StartWithPaths` checked for `ha.pid` with `os.Stat`, which bypassed this logic;
it now uses `ReadPIDFile` as well.

Related to #5087
Related to #5075

Signed-off-by: Eugene Kalinin <e.v.kalinin@gmail.com>

cmd/limactl/hostagent.go

@@ -12,7 +12,6 @@ import (
 	"os"
 	"os/signal"
 	"runtime"
-	"strconv"
 	"syscall"
 
 	"github.com/sirupsen/logrus"
@@ -52,7 +51,7 @@ func hostagentAction(cmd *cobra.Command, args []string) error {
 		} else if err != nil {
 			return fmt.Errorf("failed to determine if another hostagent is running: %w", err)
 		}
-		if err := os.WriteFile(pidfile, []byte(strconv.Itoa(os.Getpid())+"\n"), 0o644); err != nil {
+		if err := store.WritePIDFile(pidfile, os.Getpid()); err != nil {
 			return err
 		}
 		defer os.RemoveAll(pidfile)

pkg/driver/krunkit/krunkit_driver_darwin_arm64.go

@@ -29,6 +29,7 @@ import (
 	"github.com/lima-vm/lima/v2/pkg/networks/usernet"
 	"github.com/lima-vm/lima/v2/pkg/osutil"
 	"github.com/lima-vm/lima/v2/pkg/ptr"
+	"github.com/lima-vm/lima/v2/pkg/store"
 )
 
 const (
@@ -107,7 +108,7 @@ func (l *LimaKrunkitDriver) Start(ctx context.Context) (chan error, error) {
 	}
 
 	pidPath := filepath.Join(l.Instance.Dir, filenames.PIDFile(*l.Instance.Config.VMType))
-	if err := os.WriteFile(pidPath, fmt.Appendf(nil, "%d\n", krunkitCmd.Process.Pid), 0o644); err != nil {
+	if err := store.WritePIDFile(pidPath, krunkitCmd.Process.Pid); err != nil {
 		logrus.WithError(err).Warn("Failed to write PID file")
 	}
 

pkg/driver/vz/vm_darwin.go

@@ -98,7 +98,7 @@ func startVM(ctx context.Context, inst *limatype.Instance, sshLocalPort int, onV
 						logrus.Errorf("pidfile %#q already exists", pidFile)
 						sendErrCh <- err
 					}
-					if err := os.WriteFile(pidFile, []byte(strconv.Itoa(os.Getpid())+"\n"), 0o644); err != nil {
+					if err := store.WritePIDFile(pidFile, os.Getpid()); err != nil {
 						logrus.Errorf("error writing to pid fil %#q", pidFile)
 						sendErrCh <- err
 					}

pkg/instance/start.go

@@ -222,7 +222,11 @@ func Prepare(ctx context.Context, inst *limatype.Instance, guestAgent string) (*
 // StartWithPaths calls Prepare by itself, so you do not need to call Prepare manually before calling Start.
 func StartWithPaths(ctx context.Context, inst *limatype.Instance, launchHostAgentForeground, showProgress bool, limactl, guestAgent string) error {
 	haPIDPath := filepath.Join(inst.Dir, filenames.HostAgentPID)
-	if _, err := os.Stat(haPIDPath); !errors.Is(err, os.ErrNotExist) {
+	// ReadPIDFile removes the PID file when it was left behind by a process that is not
+	// running anymore, or by a previous boot of the host.
+	if haPID, err := store.ReadPIDFile(haPIDPath); err != nil {
+		return err
+	} else if haPID != 0 {
 		return fmt.Errorf("instance %#q seems running (hint: remove %#q if the instance is not actually running)", inst.Name, haPIDPath)
 	}
 	logrus.Infof("Starting the instance %#q with %s VM driver %#q", inst.Name, registry.CheckInternalOrExternal(inst.VMType), inst.VMType)

pkg/limatype/filenames/filenames.go

@@ -65,6 +65,7 @@ const (
 	HostAgentStdoutLog      = "ha.stdout.log"
 	HostAgentStderrLog      = "ha.stderr.log"
 	ExternalDriverStderrLog = "driver.stderr.log"
+	BootSession             = "boot-session.tmp" // boot of the host during which the PID files in the same directory were written
 	VzIdentifier            = "vz-identifier"
 	VzHwModel               = "vz-hwmodel"       // macOS guests only
 	VzAux                   = "vz-aux"           // macOS guests only

pkg/osutil/bootsession.go

@@ -0,0 +1,10 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package osutil
+
+import "errors"
+
+// ErrBootSessionNotSupported is returned by BootSessionID on platforms that do not expose
+// an identifier of the current boot.
+var ErrBootSessionNotSupported = errors.New("boot session ID is not supported on this platform")

pkg/osutil/bootsession_darwin.go

@@ -0,0 +1,13 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package osutil
+
+import "golang.org/x/sys/unix"
+
+// BootSessionID returns an identifier of the current boot of the host.
+// The identifier changes on every reboot, so it can be used to detect state that was left
+// behind by a previous boot.
+func BootSessionID() (string, error) {
+	return unix.Sysctl("kern.bootsessionuuid")
+}

pkg/osutil/bootsession_linux.go

@@ -0,0 +1,20 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package osutil
+
+import (
+	"os"
+	"strings"
+)
+
+// BootSessionID returns an identifier of the current boot of the host.
+// The identifier changes on every reboot, so it can be used to detect state that was left
+// behind by a previous boot.
+func BootSessionID() (string, error) {
+	b, err := os.ReadFile("/proc/sys/kernel/random/boot_id")
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSpace(string(b)), nil
+}

pkg/osutil/bootsession_others.go

@@ -0,0 +1,11 @@
+//go:build !darwin && !linux
+
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package osutil
+
+// BootSessionID returns an identifier of the current boot of the host.
+func BootSessionID() (string, error) {
+	return "", ErrBootSessionNotSupported
+}

pkg/store/instance.go

@@ -197,9 +197,22 @@ func inspectStatusWithPIDFiles(instDir string, inst *limatype.Instance, y *limat
 	}
 }
 
-// ReadPIDFile returns 0 if the PID file does not exist or the process has already terminated
-// (in which case the PID file will be removed).
+// ReadPIDFile returns 0 if the PID file does not exist, was written during a previous boot
+// of the host, or the process has already terminated (in which case the PID file will be
+// removed).
 func ReadPIDFile(path string) (int, error) {
+	// The boot is checked before the PID is read, so that a PID file of a previous boot is
+	// never trusted, not even when another process refreshes the marker in between.
+	if previousBoot, err := pidFileFromPreviousBoot(path); err != nil {
+		return 0, err
+	} else if previousBoot {
+		// The PID was recorded before the last reboot, so it is meaningless now: it may
+		// have been reused by an unrelated process, which must not be signaled. Removing
+		// the file is left to WritePIDFile, which knows that no other process is writing
+		// a PID file of the current boot into the same directory.
+		logrus.Debugf("Ignoring PID file %#q left behind by a previous boot", path)
+		return 0, nil
+	}
 	b, err := os.ReadFile(path)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {

pkg/store/pidfile.go

@@ -0,0 +1,128 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package store
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/sirupsen/logrus"
+
+	"github.com/lima-vm/lima/v2/pkg/limatype/filenames"
+	"github.com/lima-vm/lima/v2/pkg/osutil"
+)
+
+// pidFileSuffix is the suffix of the PID files of an instance.
+const pidFileSuffix = ".pid"
+
+// bootSessionID is a variable so that it can be replaced in the tests.
+var bootSessionID = osutil.BootSessionID
+
+// currentBootSessionID returns the ID of the current boot of the host, or an empty string
+// on the platforms that do not provide one. Any other failure is returned as an error:
+// guessing would either void a PID file that is current, or trust one that is not.
+func currentBootSessionID() (string, error) {
+	id, err := bootSessionID()
+	if errors.Is(err, osutil.ErrBootSessionNotSupported) {
+		// PID files are handled the way they were before the marker was introduced.
+		return "", nil
+	}
+	if err != nil {
+		return "", fmt.Errorf("failed to determine the boot session ID: %w", err)
+	}
+	return id, nil
+}
+
+// WritePIDFile writes pid to path.
+//
+// The directory of path holds a marker with the boot session ID of the host, so that
+// ReadPIDFile can tell whether a PID file was written during the current boot. When the
+// marker is from a previous boot, every PID file of the directory is removed before the
+// marker is refreshed: those PIDs are meaningless, and refreshing the marker would
+// otherwise make them look current.
+func WritePIDFile(path string, pid int) error {
+	dir := filepath.Dir(path)
+	current, err := currentBootSessionID()
+	if err != nil {
+		return err
+	}
+	if current != "" {
+		stale, err := staleBootSession(dir, current)
+		if err != nil {
+			return err
+		}
+		if stale {
+			if err := removePIDFiles(dir); err != nil {
+				return err
+			}
+		}
+		if err := writeBootSessionMarker(dir, current); err != nil {
+			return err
+		}
+	}
+	return writePIDFileAtomically(path, pid)
+}
+
+// writePIDFileAtomically renames the PID file into place, so that a reader never sees a partially
+// written one.
+func writePIDFileAtomically(path string, pid int) error {
+	tmpPath := path + ".tmp"
+	if err := os.WriteFile(tmpPath, fmt.Appendf(nil, "%d\n", pid), 0o644); err != nil {
+		return err
+	}
+	if err := os.Rename(tmpPath, path); err != nil {
+		_ = os.Remove(tmpPath)
+		return err
+	}
+	return nil
+}
+
+// pidFileFromPreviousBoot returns true when the PID file at path was written during a
+// previous boot of the host.
+func pidFileFromPreviousBoot(path string) (bool, error) {
+	current, err := currentBootSessionID()
+	if err != nil || current == "" {
+		return false, err
+	}
+	return staleBootSession(filepath.Dir(path), current)
+}
+
+// staleBootSession returns true when dir holds a boot session marker of a boot other than
+// the current one.
+func staleBootSession(dir, current string) (bool, error) {
+	b, err := os.ReadFile(filepath.Join(dir, filenames.BootSession))
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			// No marker: the PID files were written by an older version of Lima.
+			return false, nil
+		}
+		return false, err
+	}
+	return strings.TrimSpace(string(b)) != current, nil
+}
+
+func writeBootSessionMarker(dir, id string) error {
+	return os.WriteFile(filepath.Join(dir, filenames.BootSession), []byte(id+"\n"), 0o644)
+}
+
+func removePIDFiles(dir string) error {
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return err
+	}
+	for _, entry := range entries {
+		if entry.IsDir() || !strings.HasSuffix(entry.Name(), pidFileSuffix) {
+			continue
+		}
+		path := filepath.Join(dir, entry.Name())
+		logrus.Infof("Removing PID file %#q left behind by a previous boot", path)
+		if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
+			return err
+		}
+	}
+	return nil
+}

pkg/store/pidfile_test.go

@@ -0,0 +1,141 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package store
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"gotest.tools/v3/assert"
+
+	"github.com/lima-vm/lima/v2/pkg/limatype"
+	"github.com/lima-vm/lima/v2/pkg/limatype/filenames"
+	"github.com/lima-vm/lima/v2/pkg/osutil"
+)
+
+// fakeBootSessionID replaces the source of the boot session ID for the duration of the test.
+func fakeBootSessionID(t *testing.T, id string, err error) {
+	t.Helper()
+	orig := bootSessionID
+	bootSessionID = func() (string, error) { return id, err }
+	t.Cleanup(func() { bootSessionID = orig })
+}
+
+func TestWritePIDFile(t *testing.T) {
+	fakeBootSessionID(t, "boot-1", nil)
+	dir := t.TempDir()
+	path := filepath.Join(dir, filenames.HostAgentPID)
+	assert.NilError(t, WritePIDFile(path, os.Getpid()))
+
+	assert.Assert(t, osutil.FileExists(filepath.Join(dir, filenames.BootSession)))
+
+	pid, err := ReadPIDFile(path)
+	assert.NilError(t, err)
+	assert.Equal(t, pid, os.Getpid())
+}
+
+func TestWritePIDFileRemovesPIDFilesFromPreviousBoot(t *testing.T) {
+	fakeBootSessionID(t, "boot-1", nil)
+	dir := t.TempDir()
+	haPath := filepath.Join(dir, filenames.HostAgentPID)
+	driverPath := filepath.Join(dir, filenames.PIDFile(limatype.QEMU))
+	assert.NilError(t, WritePIDFile(haPath, os.Getpid()))
+	assert.NilError(t, WritePIDFile(driverPath, os.Getpid()))
+
+	// Writing a PID file during a new boot voids every PID file of the directory, so that
+	// refreshing the marker cannot make a PID file of the previous boot look current.
+	fakeBootSessionID(t, "boot-2", nil)
+	assert.NilError(t, WritePIDFile(haPath, os.Getpid()))
+
+	_, err := os.Stat(driverPath)
+	assert.Assert(t, errors.Is(err, os.ErrNotExist))
+
+	pid, err := ReadPIDFile(haPath)
+	assert.NilError(t, err)
+	assert.Equal(t, pid, os.Getpid())
+}
+
+func TestReadPIDFileFromPreviousBoot(t *testing.T) {
+	fakeBootSessionID(t, "boot-1", nil)
+	dir := t.TempDir()
+	path := filepath.Join(dir, filenames.HostAgentPID)
+	assert.NilError(t, WritePIDFile(path, os.Getpid()))
+
+	// After a reboot the PID may belong to an unrelated process, so the PID file has to be
+	// ignored even though the PID is alive.
+	fakeBootSessionID(t, "boot-2", nil)
+
+	pid, err := ReadPIDFile(path)
+	assert.NilError(t, err)
+	assert.Equal(t, pid, 0)
+	// Removing it is left to the next writer, so that a PID file that another process is
+	// writing at this very moment cannot be removed by a reader.
+	assert.Assert(t, osutil.FileExists(path))
+}
+
+func TestReadPIDFileCorruptedFromPreviousBoot(t *testing.T) {
+	fakeBootSessionID(t, "boot-1", nil)
+	dir := t.TempDir()
+	path := filepath.Join(dir, filenames.HostAgentPID)
+	assert.NilError(t, WritePIDFile(path, os.Getpid()))
+	// A PID file that was truncated when the host was powered off.
+	assert.NilError(t, os.WriteFile(path, nil, 0o644))
+
+	fakeBootSessionID(t, "boot-2", nil)
+
+	pid, err := ReadPIDFile(path)
+	assert.NilError(t, err)
+	assert.Equal(t, pid, 0)
+}
+
+func TestReadPIDFileWithoutBootSessionMarker(t *testing.T) {
+	// PID files written by an older version of Lima have no marker, and are handled as
+	// before: only the liveness of the process matters.
+	fakeBootSessionID(t, "boot-1", nil)
+	dir := t.TempDir()
+	path := filepath.Join(dir, filenames.HostAgentPID)
+	assert.NilError(t, os.WriteFile(path, fmt.Appendf(nil, "%d\n", os.Getpid()), 0o644))
+
+	pid, err := ReadPIDFile(path)
+	assert.NilError(t, err)
+	assert.Equal(t, pid, os.Getpid())
+}
+
+func TestWritePIDFileWhenBootSessionIDFails(t *testing.T) {
+	// The PID file must not be written when it cannot be tied to the current boot:
+	// a marker of a previous boot would otherwise void a PID file that is current.
+	fakeBootSessionID(t, "", errors.New("sysctl failed"))
+	dir := t.TempDir()
+	path := filepath.Join(dir, filenames.HostAgentPID)
+
+	assert.ErrorContains(t, WritePIDFile(path, os.Getpid()), "sysctl failed")
+	assert.Assert(t, !osutil.FileExists(path))
+}
+
+func TestReadPIDFileWhenBootSessionIDFails(t *testing.T) {
+	fakeBootSessionID(t, "boot-1", nil)
+	dir := t.TempDir()
+	path := filepath.Join(dir, filenames.HostAgentPID)
+	assert.NilError(t, WritePIDFile(path, os.Getpid()))
+
+	fakeBootSessionID(t, "", errors.New("sysctl failed"))
+	_, err := ReadPIDFile(path)
+	assert.ErrorContains(t, err, "sysctl failed")
+}
+
+func TestWritePIDFileWithoutBootSessionSupport(t *testing.T) {
+	fakeBootSessionID(t, "", osutil.ErrBootSessionNotSupported)
+	dir := t.TempDir()
+	path := filepath.Join(dir, filenames.HostAgentPID)
+	assert.NilError(t, WritePIDFile(path, os.Getpid()))
+
+	assert.Assert(t, !osutil.FileExists(filepath.Join(dir, filenames.BootSession)))
+
+	pid, err := ReadPIDFile(path)
+	assert.NilError(t, err)
+	assert.Equal(t, pid, os.Getpid())
+}