issue 1841/S7: name the processes still mapping a replaced file and fail the check on a DKMS module not built for an installed kernel, so a machine is never left unable to load its drivers unnoticed

2026-10-22 14:39:29 +02:00
parent 75f3f52589
commit 2b2517e4a5
8 changed files with 320 additions and 2 deletions
@@ -50,6 +50,7 @@ func (u *Upgrades) AfterCheck(run Run, rec *Record) Check {
 	}
 	c.RunningKernel, c.InstalledKernel, c.RestartNeeded = u.restartNeeded()
 	c.Passed = succeeded && healthy && len(c.NewlyFailed) == 0
+	u.afterCheckII(&c)
 	return c
 }

@@ -35,6 +35,7 @@ func checkMachine(failed, linuxNow string) *Machine {
 	m := machine(byLine(map[string]Ran{
 		"systemctl list-units --state=failed --plain --no-legend --no-pager": {Stdout: failed},
 		"pacman -Qq":                    {Stdout: "bash\nlinux\nlinux-firmware\nlinux-lts\n"},
+		"dkms status":                   {Status: 127, Err: "ENOENT"},
 		"pacman -Qlq -- linux":          {Stdout: "/usr/lib/modules/" + linuxNow + "/vmlinuz\n"},
 		"pacman -Qlq -- linux-firmware": {Stdout: "/usr/lib/firmware/\n"},
 		"pacman -Qlq -- linux-lts":      {Stdout: "/usr/lib/modules/6.18.54-2-lts/vmlinuz\n"},
@@ -26,8 +26,8 @@ var busFields = map[string][]string{
 	"Step":             {"step", "at", "why", "held", "unit", "ignored", "plan", "exit", "error", "changes", "check"},
 	"HeldKernel":       {"package", "version", "module", "bound"},
 	"Changes":          {"added", "removed", "changed"},
-	"Check": {"passed", "pacnew", "pacsave", "news", "news_error", "mapped", "newly_failed", "restart_needed", "running_kernel",
-		"installed_kernel", "unbuilt"},
+	"Check": {"passed", "pacnew", "pacsave", "news", "news_error", "mapped", "mapped_error", "newly_failed", "restart_needed", "running_kernel",
+		"installed_kernel", "unbuilt", "dkms_error"},
 	"NewsTitle":        {"title", "link", "published"},
 	"Mapped":           {"unit", "container", "executable"},
 	"DKMSBuild":        {"module", "version", "kernel"},
@@ -0,0 +1,173 @@
+package main
+
+// The after-upgrade check, its second part (novox/hq ADR 0508 §4): the processes still mapping a file the upgrade
+// replaced, named by their unit or container and their executable's path, never their command line or
+// environment; and whether every DKMS module is built for every installed kernel. Reading another process's
+// mappings needs root, so the root helper reads them (ADR 0510's `maps`), with MappedDeleted below.
+
+import (
+	"os"
+	"path"
+	"regexp"
+	"sort"
+	"strconv"
+	"strings"
+)
+
+// ProcFS is the process table as MappedDeleted reads it: the machine's /proc, or a stand-in in a test.
+type ProcFS interface {
+	ReadDir(dir string) ([]string, error)
+	ReadFile(name string) ([]byte, error)
+	Readlink(name string) (string, error)
+}
+
+// osProc is the machine's own /proc.
+type osProc struct{}
+
+func (osProc) ReadDir(dir string) ([]string, error) {
+	entries, err := os.ReadDir(dir)
+	var names []string
+	for _, e := range entries {
+		names = append(names, e.Name())
+	}
+	return names, err
+}
+func (osProc) ReadFile(name string) ([]byte, error) { return os.ReadFile(name) }
+func (osProc) Readlink(name string) (string, error) { return os.Readlink(name) }
+
+// replacedPrefixes are where a package's files are: a mapping deleted elsewhere (a temporary file, shared
+// memory) is no file an upgrade replaced.
+var replacedPrefixes = []string{"/usr/", "/opt/"}
+
+var containerScope = regexp.MustCompile(`(?:docker|libpod|crio|cri-containerd)-([0-9a-f]{12})[0-9a-f]*\.scope$`)
+
+// cgroupOwner names a process's unit or container from its cgroup line `0::/system.slice/x.service`.
+func cgroupOwner(cgroup string) (unit, container string) {
+	for _, l := range strings.Split(cgroup, "\n") {
+		_, p, ok := strings.Cut(l, "::")
+		if !ok {
+			continue
+		}
+		parts := strings.Split(strings.Trim(p, "/"), "/")
+		for i := len(parts) - 1; i >= 0; i-- {
+			if g := containerScope.FindStringSubmatch(parts[i]); g != nil {
+				return "", g[1]
+			}
+			if i > 0 && parts[i-1] == "docker" && len(parts[i]) >= 12 {
+				return "", parts[i][:12]
+			}
+		}
+		for i := len(parts) - 1; i >= 0; i-- {
+			if strings.HasSuffix(parts[i], ".service") || strings.HasSuffix(parts[i], ".scope") {
+				return parts[i], ""
+			}
+		}
+	}
+	return "", ""
+}
+
+// MappedDeleted reads the process table under procRoot for processes mapping a deleted package file, each named
+// once by its unit or container and its executable, sorted.
+func MappedDeleted(fs ProcFS, procRoot string) ([]Mapped, error) {
+	names, err := fs.ReadDir(procRoot)
+	if err != nil {
+		return nil, err
+	}
+	seen := map[Mapped]bool{}
+	out := []Mapped{}
+	for _, name := range names {
+		if _, err := strconv.Atoi(name); err != nil {
+			continue
+		}
+		dir := path.Join(procRoot, name)
+		maps, err := fs.ReadFile(dir + "/maps")
+		if err != nil {
+			continue // the process ended meanwhile
+		}
+		replaced := false
+		for _, l := range strings.Split(string(maps), "\n") {
+			if !strings.HasSuffix(l, " (deleted)") {
+				continue
+			}
+			f := strings.Fields(strings.TrimSuffix(l, " (deleted)"))
+			if len(f) < 6 {
+				continue
+			}
+			for _, p := range replacedPrefixes {
+				if strings.HasPrefix(f[5], p) {
+					replaced = true
+				}
+			}
+		}
+		if !replaced {
+			continue
+		}
+		exe, _ := fs.Readlink(dir + "/exe")
+		cgroup, _ := fs.ReadFile(dir + "/cgroup")
+		unit, container := cgroupOwner(string(cgroup))
+		m := Mapped{Unit: unit, Container: container, Executable: strings.TrimSuffix(exe, " (deleted)")}
+		if !seen[m] {
+			seen[m] = true
+			out = append(out, m)
+		}
+	}
+	sort.Slice(out, func(i, j int) bool {
+		a, b := out[i], out[j]
+		if a.Unit+a.Container != b.Unit+b.Container {
+			return a.Unit+a.Container < b.Unit+b.Container
+		}
+		return a.Executable < b.Executable
+	})
+	return out, nil
+}
+
+// unbuiltDKMS is every DKMS module built for some kernel and not for an installed one its own bound allows.
+func (m *Machine) unbuiltDKMS() ([]DKMSBuild, error) {
+	modules, err := m.DKMSModules()
+	if err != nil {
+		return nil, err
+	}
+	kernels, err := m.installedKernels()
+	if err != nil {
+		return nil, err
+	}
+	out := []DKMSBuild{}
+	for _, mod := range modules {
+		if len(mod.Kernels) == 0 {
+			continue // added and never built: not a module this machine uses
+		}
+		bound := m.dkmsBound(mod)
+		for _, k := range kernels {
+			if contains(mod.Kernels, k) {
+				continue
+			}
+			if held, _ := heldBy(bound, k); held != "" {
+				continue
+			}
+			out = append(out, DKMSBuild{Module: mod.Module, Version: mod.Version, Kernel: k})
+		}
+	}
+	return out, nil
+}
+
+// afterCheckII fills the check's second part and judges it: a DKMS module not built for a kernel fails it;
+// a process still mapping a replaced file is said and does not.
+func (u *Upgrades) afterCheckII(c *Check) {
+	if u.Root == nil {
+		c.MappedError = "no root helper to read the processes' mapped files"
+	} else if mapped, err := u.Root.Maps(); err != nil {
+		c.MappedError = err.Error()
+	} else {
+		c.Mapped = mapped
+	}
+	unbuilt, err := u.Machine.unbuiltDKMS()
+	if err != nil {
+		c.DKMSError = err.Error()
+		c.Passed = false
+		return
+	}
+	c.Unbuilt = unbuilt
+	if len(unbuilt) > 0 {
+		c.Passed = false
+	}
+}
@@ -0,0 +1,131 @@
+package main
+
+// The after-upgrade check, its second part (novox/hq ADR 0508 §4; acceptance 1841/AC5): processes still mapping a
+// replaced file, by unit or container and executable, never a command line or environment; DKMS built for every
+// installed kernel.
+
+import (
+	"errors"
+	"strings"
+	"testing"
+)
+
+// standInProc is a process table in memory; it records every file read, so nothing else is read.
+type standInProc struct {
+	files map[string]string
+	links map[string]string
+	read  []string
+}
+
+func (p *standInProc) ReadDir(dir string) ([]string, error) {
+	seen := map[string]bool{}
+	var names []string
+	for f := range p.files {
+		if rest, ok := strings.CutPrefix(f, dir+"/"); ok {
+			n, _, _ := strings.Cut(rest, "/")
+			if !seen[n] {
+				seen[n] = true
+				names = append(names, n)
+			}
+		}
+	}
+	return names, nil
+}
+
+func (p *standInProc) ReadFile(name string) ([]byte, error) {
+	p.read = append(p.read, name)
+	if s, ok := p.files[name]; ok {
+		return []byte(s), nil
+	}
+	return nil, errNoFile
+}
+
+func (p *standInProc) Readlink(name string) (string, error) {
+	if s, ok := p.links[name]; ok {
+		return s, nil
+	}
+	return "", errNoFile
+}
+
+const libcDeleted = "7f2a1c000000-7f2a1c028000 r--p 00000000 103:02 5513 /usr/lib/libc.so.6 (deleted)\n"
+
+func TestAProcessMappingAReplacedFileIsNamedByItsUnitOrContainerAndExecutableOnly(t *testing.T) {
+	p := &standInProc{files: map[string]string{
+		"/proc/1/maps":      libcDeleted,
+		"/proc/1/cgroup":    "0::/init.scope\n",
+		"/proc/200/maps":    "5600-5700 r-xp 00000000 103:02 77 /usr/bin/sshd\n" + strings.Replace(libcDeleted, "libc.so.6", "libcrypto.so.3", 1),
+		"/proc/200/cgroup":  "0::/system.slice/sshd.service\n",
+		"/proc/201/maps":    libcDeleted,
+		"/proc/201/cgroup":  "0::/system.slice/sshd.service\n",
+		"/proc/300/maps":    libcDeleted,
+		"/proc/300/cgroup":  "0::/system.slice/docker-0123456789abcdef0123456789abcdef.scope\n",
+		"/proc/400/maps":    "7f00-7f10 rw-s 00000000 00:1a 9 /dev/shm/made-up (deleted)\n",
+		"/proc/400/cgroup":  "0::/system.slice/made-up.service\n",
+		"/proc/500/maps":    "5600-5700 r-xp 00000000 103:02 77 /usr/bin/made-up\n",
+		"/proc/500/cmdline": "made-up --password=secret",
+		"/proc/500/environ": "TOKEN=secret",
+		"/proc/self/maps":   libcDeleted,
+	}, links: map[string]string{
+		"/proc/1/exe": "/usr/lib/systemd/systemd", "/proc/200/exe": "/usr/bin/sshd (deleted)", "/proc/201/exe": "/usr/bin/sshd (deleted)",
+		"/proc/300/exe": "/usr/bin/made-up-server",
+	}}
+	got, err := MappedDeleted(p, "/proc")
+	if err != nil {
+		t.Fatal(err)
+	}
+	var said []string
+	for _, m := range got {
+		said = append(said, m.Unit+"|"+m.Container+"|"+m.Executable)
+	}
+	if strings.Join(said, " ") != "|0123456789ab|/usr/bin/made-up-server init.scope||/usr/lib/systemd/systemd sshd.service||/usr/bin/sshd" {
+		t.Errorf("%v", said)
+	}
+	for _, r := range p.read {
+		if strings.HasSuffix(r, "/cmdline") || strings.HasSuffix(r, "/environ") {
+			t.Errorf("read %s: a process is named by its unit and executable, never its command line or environment", r)
+		}
+	}
+}
+
+func TestADKMSModuleNotBuiltForAnInstalledKernelFailsTheCheck(t *testing.T) {
+	m := checkMachine("", "7.2.9-arch1-1")
+	table := map[string]Ran{
+		"systemctl list-units --state=failed --plain --no-legend --no-pager": {},
+		"pacman -Qq":               {Stdout: "linux\nlinux-lts\n"},
+		"pacman -Qlq -- linux":     {Stdout: "/usr/lib/modules/7.2.9-arch1-1/vmlinuz\n"},
+		"pacman -Qlq -- linux-lts": {Stdout: "/usr/lib/modules/6.18.54-2-lts/vmlinuz\n"},
+		"dkms status": {Stdout: "zfs/2.4.4, 6.18.54-2-lts, x86_64: installed\nzfs/2.4.4, 7.2.8-arch1-2, x86_64: installed\n" +
+			"made-up-driver/1.0, 6.18.54-2-lts, x86_64: installed\nxpad/0.4: added\n"},
+	}
+	m.Run = byLine(table, nil)
+	read := m.ReadFile
+	m.ReadFile = func(path string) ([]byte, error) {
+		if path == "/usr/src/made-up-driver-1.0/dkms.conf" {
+			return []byte("BUILD_EXCLUSIVE_KERNEL=\"^6\\.\"\n"), nil
+		}
+		return read(path)
+	}
+	u := checkUpgrades(t, m)
+	u.Root = &fakeRoot{mapped: []Mapped{{Unit: "sshd.service", Executable: "/usr/bin/sshd"}}}
+	c := u.AfterCheck(checkedRun(StepStarted, StepFinished), &Record{})
+	if len(c.Unbuilt) != 1 || c.Unbuilt[0] != (DKMSBuild{Module: "zfs", Version: "2.4.4", Kernel: "7.2.9-arch1-1"}) || c.Passed {
+		t.Errorf("zfs not built for the new kernel fails the check (made-up-driver excludes it, xpad was never built): %+v", c)
+	}
+	if len(c.Mapped) != 1 || c.Mapped[0].Unit != "sshd.service" {
+		t.Errorf("mapped: %+v", c.Mapped)
+	}
+
+	table["dkms status"] = Ran{Stdout: "zfs/2.4.4, 6.18.54-2-lts, x86_64: installed\nzfs/2.4.4, 7.2.9-arch1-1, x86_64: installed\n"}
+	if c := u.AfterCheck(checkedRun(StepStarted, StepFinished), &Record{}); !c.Passed || len(c.Mapped) != 1 {
+		t.Errorf("built for every kernel passes; a mapped file is said and does not fail it: %+v", c)
+	}
+
+	u.Root = &fakeRoot{mapsErr: errors.New("the root helper's socket is not there")}
+	if c := u.AfterCheck(checkedRun(StepStarted, StepFinished), &Record{}); !strings.Contains(c.MappedError, "socket") {
+		t.Errorf("unread mappings are said: %+v", c)
+	}
+	table["dkms status"] = Ran{Status: 1, Stderr: "dkms: permission denied\n"}
+	if c := u.AfterCheck(checkedRun(StepStarted, StepFinished), &Record{}); c.Passed || c.DKMSError == "" {
+		t.Errorf("unread DKMS builds do not pass: %+v", c)
+	}
+}
@@ -235,6 +235,8 @@ type Check struct {
 	NewsError string `json:"news_error,omitempty"`
 	// The units and containers whose processes still map a file the upgrade replaced.
 	Mapped []Mapped `json:"mapped"`
+	// Why the mapped files could not be read, when they could not.
+	MappedError string `json:"mapped_error,omitempty"`
 	// The units failed now that were not failed in the record.
 	NewlyFailed []string `json:"newly_failed"`
 	// Whether a restart is needed: the running kernel is not the installed version of its package.
@@ -243,6 +245,8 @@ type Check struct {
 	InstalledKernel string `json:"installed_kernel"`
 	// The DKMS modules not built for an installed kernel.
 	Unbuilt []DKMSBuild `json:"unbuilt"`
+	// Why the DKMS builds could not be read, when they could not; the check then does not pass.
+	DKMSError string `json:"dkms_error,omitempty"`
 }

 // NewsTitle is one item of the distribution's news, without its text.
@@ -18,6 +18,8 @@ type fakeRoot struct {
 	started []string
 	fail    error
 	journal []string
+	mapped  []Mapped
+	mapsErr error
 }

 func (f *fakeRoot) Transaction(kind, run string, ignore []string) error {
@@ -30,6 +32,8 @@ func (f *fakeRoot) Transaction(kind, run string, ignore []string) error {

 func (f *fakeRoot) Journal(unit string, n int) ([]string, error) { return f.journal, nil }

+func (f *fakeRoot) Maps() ([]Mapped, error) { return f.mapped, f.mapsErr }
+
 // keeperBed is one node's keeper over states in memory and a machine of fakes: a unit's state, the lock and
 // pacman's log are set by the test as the transaction goes.
 type keeperBed struct {
@@ -75,6 +79,8 @@ func newKeeperBed(t *testing.T) *keeperBed {
 			return Ran{}
 		case line == "dkms status":
 			return Ran{Stdout: zfsStatus}
+		case line == "pacman -Qq":
+			return Ran{Stdout: "bash\nlinux\n"}
 		case line == "pacman -Qlq -- linux":
 			return files("/usr/lib/modules/7.2.8-arch1-2/vmlinuz")
 		case line == "pacman -Qlq -- linux-headers":
@@ -41,6 +41,8 @@ type RootActs interface {
 	Transaction(kind, run string, ignore []string) error
 	// Journal is the last n lines a transaction unit logged.
 	Journal(unit string, n int) ([]string, error)
+	// Maps is every process still mapping a replaced file, by its unit or container and its executable.
+	Maps() ([]Mapped, error)
 }

 // TransactionUnit is the unit a run's transaction runs as: named by the run, so a keeper started again finds it.