agent v0.36.4: serialize AttachDrive/DetachDrive (no double-bind TOCTOU race)

A GuestBinder mutex prevents a concurrent reconnect + periodic reconcile from
both passing isHostMountpoint and double-binding a stable path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-15 20:19:07 +02:00
parent cff9284453
commit a356d6def4
4 changed files with 16 additions and 1 deletions
+6
View File
@@ -3,6 +3,12 @@
All notable changes to **felhom-agent** are recorded here. Update on every code All notable changes to **felhom-agent** are recorded here. Update on every code
change that gets pushed. change that gets pushed.
## v0.36.4 — serialize AttachDrive/DetachDrive (no double-bind race) (2026-06-15)
A mutex on GuestBinder serializes AttachDrive/DetachDrive so a controller-triggered reconnect and the
agent`s periodic reconcile can no longer both pass the isHostMountpoint check and double-bind the same
stable path (a TOCTOU race observed live as 2 stacked binds during rapid eject/reconnect).
## v0.36.3 — DetachDrive loop-umounts stacked binds (2026-06-15) ## v0.36.3 — DetachDrive loop-umounts stacked binds (2026-06-15)
DetachDrive now umounts ALL stacked binds at a stable path (bounded loop), not just one layer — so an DetachDrive now umounts ALL stacked binds at a stable path (bounded loop), not just one layer — so an
+1 -1
View File
@@ -44,7 +44,7 @@ import (
// version is the agent version. Overridable at build time with // version is the agent version. Overridable at build time with
// -ldflags "-X main.version=<v>"; defaults to the in-repo CHANGELOG version. // -ldflags "-X main.version=<v>"; defaults to the in-repo CHANGELOG version.
var version = "0.36.3" var version = "0.36.4"
// runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook <vmid> <phase>`). On the // runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook <vmid> <phase>`). On the
// pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots // pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots
+5
View File
@@ -5,6 +5,7 @@ import (
"fmt" "fmt"
"log/slog" "log/slog"
"strconv" "strconv"
"sync"
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
) )
@@ -35,6 +36,10 @@ const felhomDataNS = "felhom-data"
type GuestBinder struct { type GuestBinder struct {
runner proxmox.Runner runner proxmox.Runner
logger *slog.Logger logger *slog.Logger
// mountMu serializes AttachDrive/DetachDrive so a concurrent reconnect (controller-triggered) and the
// agent's periodic reconcile can't both pass the isHostMountpoint check and double-bind the same
// stable path (a TOCTOU race — observed live as 2 stacked binds).
mountMu sync.Mutex
} }
// NewGuestBinder builds a binder over the given root-CLI runner. // NewGuestBinder builds a binder over the given root-CLI runner.
+4
View File
@@ -151,6 +151,8 @@ func (b *GuestBinder) installSharedParentUnit(ctx context.Context) error {
// (umount + mount) to fire a fresh propagation event into the current guest namespace. Idempotent when // (umount + mount) to fire a fresh propagation event into the current guest namespace. Idempotent when
// the guest already sees it. // the guest already sees it.
func (b *GuestBinder) AttachDrive(ctx context.Context, vmid int, where string) (string, error) { func (b *GuestBinder) AttachDrive(ctx context.Context, vmid int, where string) (string, error) {
b.mountMu.Lock() // serialize vs a concurrent DetachDrive/AttachDrive (no double-bind TOCTOU)
defer b.mountMu.Unlock()
stable := StablePathForRaw(where) stable := StablePathForRaw(where)
if stable == "" { if stable == "" {
return "", fmt.Errorf("guest-attach: %q is not a /mnt/<name> mount", where) return "", fmt.Errorf("guest-attach: %q is not a /mnt/<name> mount", where)
@@ -289,6 +291,8 @@ func hostBtime() string {
// leaving the bare HOST-ROOT-owned stable dir → fail-closed (the guest can't write to it even as root, // leaving the bare HOST-ROOT-owned stable dir → fail-closed (the guest can't write to it even as root,
// since host uid 0 is unmapped). No pct, no reboot. Idempotent: a non-mountpoint is a no-op. // since host uid 0 is unmapped). No pct, no reboot. Idempotent: a non-mountpoint is a no-op.
func (b *GuestBinder) DetachDrive(ctx context.Context, where string) error { func (b *GuestBinder) DetachDrive(ctx context.Context, where string) error {
b.mountMu.Lock() // serialize vs a concurrent AttachDrive (so detach can't race a re-bind)
defer b.mountMu.Unlock()
stable := StablePathForRaw(where) stable := StablePathForRaw(where)
if stable == "" { if stable == "" {
return fmt.Errorf("guest-detach: %q is not a /mnt/<name> mount", where) return fmt.Errorf("guest-detach: %q is not a /mnt/<name> mount", where)