Merge pull request 'power: the watcher checks its lock is logind's, and takes it again when it is not' (#288) from fix/power-watcher-verifies-its-lock into main

modules/power/cmd/power/login1.go

@@ -1,7 +1,9 @@
 package main
 
 import (
+	"fmt"
 	"os"
+	"strings"
 
 	"github.com/godbus/dbus/v5"
 )
@@ -56,9 +58,29 @@ func (l *systemLogin1) Inhibit(what, who, why string) (*os.File, error) {
 	if err != nil {
 		return nil, err
 	}
+	// What came back must be logind's reference to the lock. Without descriptor passing on the
+	// connection the value is an index, not a descriptor — 0 is the bundle's own stdin, its channel to
+	// the runtime — and closing it at the next sleep would cut the module off. Never wrapped unchecked.
+	if !IsInhibitor(int(fd)) {
+		return nil, fmt.Errorf("logind answered descriptor %d, which is not an inhibitor reference (%s)", int(fd), fdTarget(int(fd)))
+	}
 	return os.NewFile(uintptr(fd), "logind-inhibitor"), nil
 }
 
+// fdTarget is what one of this process's descriptors points at.
+func fdTarget(fd int) string {
+	target, err := os.Readlink(fmt.Sprintf("/proc/self/fd/%d", fd))
+	if err != nil {
+		return err.Error()
+	}
+	return target
+}
+
+// IsInhibitor is whether a descriptor of this process is a logind inhibitor's reference.
+func IsInhibitor(fd int) bool {
+	return fd > 2 && strings.HasPrefix(fdTarget(fd), "/run/systemd/inhibit/")
+}
+
 func (l *systemLogin1) Signals() <-chan Signal { return l.out }
 
 func (l *systemLogin1) Close() { l.conn.Close() }

modules/power/cmd/power/watcher.go

@@ -166,6 +166,24 @@ func (w *Watcher) inhibit(l Login1) {
 	w.lock = f
 }
 
+// stillHeld says whether the lock's descriptor is still logind's reference; replaceable in tests.
+var stillHeld = func(f *os.File) bool { return IsInhibitor(int(f.Fd())) }
+
+// verify takes the lock again when it is no longer held, and says so: a lock lost silently would
+// let the machine sleep without a word on the bus.
+func (w *Watcher) verify(l Login1) {
+	w.mu.Lock()
+	lock := w.lock
+	w.mu.Unlock()
+	if lock != nil && stillHeld(lock) {
+		return
+	}
+	w.mu.Lock()
+	w.lock = nil
+	w.mu.Unlock()
+	w.inhibit(l)
+}
+
 func (w *Watcher) release() {
 	w.mu.Lock()
 	defer w.mu.Unlock()
@@ -264,6 +282,9 @@ func (w *Watcher) Run(ctx context.Context) {
 			}
 			w.handle(l, s)
 		case <-tick.C:
+			if l != nil {
+				w.verify(l)
+			}
 			w.supply()
 			w.flush()
 		}

modules/power/cmd/power/watcher_test.go

@@ -53,6 +53,27 @@ func (b *bus) seen() []string {
 	return append([]string(nil), b.got...)
 }
 
+func init() { stillHeld = func(*os.File) bool { return true } }
+
+func TestALostLockIsTakenAgain(t *testing.T) {
+	l := &fakeLogin{}
+	w := watcher(t, machine(t, nil), &bus{})
+	w.inhibit(l)
+	held := false
+	stillHeld = func(*os.File) bool { return held }
+	defer func() { stillHeld = func(*os.File) bool { return true } }()
+	w.verify(l)
+	if l.held != 2 || !w.Snapshot().Inhibiting {
+		t.Fatalf("a lock that was no longer held was not taken again: held %d %+v", l.held, w.Snapshot())
+	}
+}
+
+func TestStdinIsNeverTakenForALock(t *testing.T) {
+	if IsInhibitor(0) || IsInhibitor(1) || IsInhibitor(2) {
+		t.Fatal("a standard descriptor was taken for logind's lock")
+	}
+}
+
 func machine(t *testing.T, files map[string]string) *Machine {
 	t.Helper()
 	root := t.TempDir()