Host power-off: a guest started after acceptance cancels it; escrow reply data race fixed
gates / gates (push) Successful in 30s
gates / gates (push) Successful in 30s
Security review of c6d9435: the other-guests check ran only at acceptance. waitJobsClear now
looks again before each step and CANCELS at once when another guest or VM runs (gate released).
Red-proofed. Also: go test -race found a data race in the escrow ceremony start reply (reads
job.Phase while the job goroutine writes it) — the reply fields are copied before the goroutine
starts; -race was red before, green 3/3 after.
This commit is contained in:
@@ -226,6 +226,9 @@ func (s *Server) handleEscrowCeremonyStart(w http.ResponseWriter, r *http.Reques
|
||||
if base == nil {
|
||||
base = context.Background()
|
||||
}
|
||||
// The reply's fields are read HERE, before the job goroutine may write job.Phase (a data race the race
|
||||
// detector caught, 2026-10-13 — the reply could show the finished phase or a torn value).
|
||||
jobID, phase := job.JobID, job.Phase
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
@@ -274,7 +277,7 @@ func (s *Server) handleEscrowCeremonyStart(w http.ResponseWriter, r *http.Reques
|
||||
"claim_ttl_s", int(escrowClaimTTL.Seconds()), "duration_ms", time.Since(start).Milliseconds())
|
||||
}()
|
||||
s.escrowDone = done // tests wait on it; production polls the status endpoint
|
||||
writeStatus(w, http.StatusAccepted, true, map[string]any{"vmid": vmid, "job_id": job.JobID, "phase": job.Phase}, "")
|
||||
writeStatus(w, http.StatusAccepted, true, map[string]any{"vmid": vmid, "job_id": jobID, "phase": phase}, "")
|
||||
}
|
||||
|
||||
// handleEscrowCeremonyStatus is GET /escrow/ceremony/status: phase + the NON-SECRET summary.
|
||||
|
||||
@@ -209,18 +209,8 @@ func (s *Server) powerOffBlocker(ctx context.Context, vmid int) (string, string)
|
||||
if s.inFlight == nil || s.staleLock == nil {
|
||||
return PowerOffUnknown, "the heavy-operation gate or the Proxmox task reader is not wired"
|
||||
}
|
||||
running, err := c.Running()
|
||||
if err != nil {
|
||||
return PowerOffUnknown, "cannot list the running guests: " + err.Error()
|
||||
}
|
||||
var others []string
|
||||
for _, v := range running {
|
||||
if v != vmid {
|
||||
others = append(others, strconv.Itoa(v))
|
||||
}
|
||||
}
|
||||
if len(others) > 0 {
|
||||
return PowerOffOtherGuests, "other guests or VMs running: " + strings.Join(others, ", ")
|
||||
if code, detail := s.otherGuestsBlocker(vmid); code != "" {
|
||||
return code, detail
|
||||
}
|
||||
if busy := s.inFlight.Busy(); busy != "" {
|
||||
switch {
|
||||
@@ -324,6 +314,24 @@ func runningGuestsFromProc(procDir string) ([]int, error) {
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// otherGuestsBlocker refuses while any guest or VM other than the caller's runs (or the list cannot be read).
|
||||
func (s *Server) otherGuestsBlocker(vmid int) (string, string) {
|
||||
running, err := s.powerOff.Running()
|
||||
if err != nil {
|
||||
return PowerOffUnknown, "cannot list the running guests: " + err.Error()
|
||||
}
|
||||
var others []string
|
||||
for _, v := range running {
|
||||
if v != vmid {
|
||||
others = append(others, strconv.Itoa(v))
|
||||
}
|
||||
}
|
||||
if len(others) > 0 {
|
||||
return PowerOffOtherGuests, "other guests or VMs running: " + strings.Join(others, ", ")
|
||||
}
|
||||
return "", ""
|
||||
}
|
||||
|
||||
// waitJobsClear waits until jobBlocker finds nothing, at most JobWait. false = still busy (or unreadable) at the end:
|
||||
// the power-off is CANCELLED — the box stays on, the gate is released, the log says why. Better a box that did not
|
||||
// switch off than a backup, update or format cut in the middle.
|
||||
@@ -332,6 +340,12 @@ func (s *Server) waitJobsClear(base context.Context, vmid int, when string) bool
|
||||
ctx, cancel := context.WithTimeout(base, c.JobWait)
|
||||
defer cancel()
|
||||
for {
|
||||
// Another guest or VM started after acceptance (security review 2026-10-13): no waiting — cancel at once.
|
||||
if code, detail := s.otherGuestsBlocker(vmid); code != "" {
|
||||
s.logger.Error("host-poweroff: CANCELLED — another guest is running now; the box stays on", "when", when,
|
||||
"reason", code, "detail", detail)
|
||||
return false
|
||||
}
|
||||
code, detail := s.jobBlocker(ctx, vmid)
|
||||
if code == "" {
|
||||
return true
|
||||
|
||||
@@ -328,3 +328,26 @@ func TestHostPowerOff_JobStartedAfterAcceptance(t *testing.T) {
|
||||
t.Fatalf("the heavy gate is still held after a cancel: %q", busy)
|
||||
}
|
||||
}
|
||||
|
||||
// A guest or VM started AFTER acceptance cancels the power-off at once (security review 2026-10-13).
|
||||
func TestHostPowerOff_OtherGuestStartedAfterAcceptance(t *testing.T) {
|
||||
r := newPowerRig(t, time.Hour)
|
||||
var started atomic.Bool
|
||||
r.srv.powerOff.Running = func() ([]int, error) {
|
||||
if started.Load() {
|
||||
return []int{8200, 341}, nil
|
||||
}
|
||||
return []int{8200}, nil
|
||||
}
|
||||
r.srv.powerOff.ReportNow = func(context.Context) bool { started.Store(true); return true } // a VM starts now
|
||||
if w := do(t, r.h, "POST", "/host/poweroff", "A", ""); w.Code != http.StatusAccepted {
|
||||
t.Fatalf("accept: %d %s", w.Code, w.Body.String())
|
||||
}
|
||||
time.Sleep(50 * time.Millisecond)
|
||||
if s := r.power.got(); len(s) != 0 {
|
||||
t.Fatalf("power-off ran although a VM started: %v", s)
|
||||
}
|
||||
if busy := r.srv.inFlight.Busy(); busy != "" {
|
||||
t.Fatalf("the heavy gate is still held after a cancel: %q", busy)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user