Skip to content

Commit fec785e

Browse files
committed
fix(microvm): the admission budget was never checked against the host's RAM
SH_MAX_COMMITTED_MB=24576 less SH_MEMORY_RESERVE_MB=4096 is a 20 GiB admission budget, and nothing in the worker reads MemTotal, so on a 16 GiB host the gate cannot refuse before the OOM killer arrives. Every other invariant in this tier fails at start rather than trusting the operator (AssertPathExists=/dev/kvm, SH_MAX_COMMITTED_MB being mandatory, Config.Normalize's cross-field checks); the one that was missing is the check against the host the unit is running on. It has a measurement consequence too: a gate that never refuses reads as confirmation of spec §7.4's prediction 1 rather than as its falsification. AssertMemory=>=23G in [Unit]. systemd compares it against physical memory (memory assigned to the container, in a container) and fails the unit at start, the same posture as AssertPathExists -- and, like it, an assertion does not trigger Restart=, so it cannot crash-loop. 23G against a 24 GiB budget rather than 24G because physical memory here is MemTotal, always somewhat below nominal RAM: a threshold equal to the budget would refuse to start on a genuine 24 GiB host. The test extracts the threshold, requires the >= comparison, normalises it to MiB and bounds it on both sides of SH_MAX_COMMITTED_MB: at least 90% of it (so raising the budget alone is a test failure, not a discovery made by the OOM killer) and no more than it (so an assertion above nominal RAM, which would refuse to start on the very host it was written for, is also a test failure). Verified red on three mutations: the assertion deleted (3 checks), SH_MAX_COMMITTED_MB raised to 65536 alone, and the threshold moved to 25G. Parse-checked against real systemd, since an unknown key would be ignored with only a warning: systemd 255 (ubuntu:24.04) and 249 (ubuntu:22.04) both accept AssertMemory and the StartLimit keys, reporting nothing but the absent ExecStart binary. This is the unit-level form of the reviewer's suggestion. A MemTotal comparison inside Config.Normalize would be the other half; internal/vmpool/ is outside this change's scope, so it is not done here. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
1 parent 0aa41d1 commit fec785e

2 files changed

Lines changed: 69 additions & 0 deletions

File tree

deploy/microvm/microvm-worker.service

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,31 @@ AssertPathExists=/dev/kvm
6868
# reintroducing.
6969
StartLimitIntervalSec=300
7070
StartLimitBurst=5
71+
# The admission budget below (SH_MAX_COMMITTED_MB) is a promise about how much memory
72+
# this host has, and nothing in the worker reads MemTotal. On a host with less RAM than
73+
# the budget the gate therefore cannot refuse before the OOM killer arrives -- it admits
74+
# VMs against memory that does not exist, and spec §7.4's prediction 1 ("replenishment
75+
# binds on process/memory count before CPU") gets "confirmed" by a gate that never
76+
# refuses at all. Every other invariant in this tier fails at start rather than trusting
77+
# the operator (AssertPathExists=/dev/kvm above, SH_MAX_COMMITTED_MB being mandatory,
78+
# Config.Normalize's cross-field checks); the one that was missing is the check against
79+
# the host this unit is actually running on. A comment is not a control.
80+
#
81+
# AssertMemory compares the amount of PHYSICAL memory in the system (in a container, the
82+
# memory assigned to it) -- systemd's ConditionMemory family, available since v244 -- so
83+
# it is the same fact the budget is asserting, read from the host. Assertion rather than
84+
# Condition deliberately: a Condition silently skips the start, an Assert fails it
85+
# loudly. And like AssertPathExists it does NOT trigger Restart=, so it cannot produce
86+
# the crash-loop the StartLimit keys above exist to bound.
87+
#
88+
# 23G against a 24 GiB SH_MAX_COMMITTED_MB, not 24G: physical memory here is MemTotal,
89+
# which is always somewhat BELOW nominal RAM (firmware and kernel reservations), so a
90+
# threshold equal to the budget would refuse to start on a genuine 24 GiB host. The band
91+
# is deliberate -- this catches "the budget is bigger than the machine" (a 16 GiB host
92+
# refuses), not a 1% discrepancy. deploy/microvm/tests/systemd-units.test.sh does that
93+
# arithmetic against SH_MAX_COMMITTED_MB below and against that tolerance, so raising
94+
# either value alone is a test failure rather than a discovery made by the OOM killer.
95+
AssertMemory=>=23G
7196

7297
[Service]
7398
Type=simple

deploy/microvm/tests/systemd-units.test.sh

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,50 @@ sh_max_committed="$(grep -oE 'SH_MAX_COMMITTED_MB=[0-9]+' "$SERVICE" | head -n1
154154
check "SH_MAX_COMMITTED_MB is set" "$([ -n "$sh_max_committed" ] && echo yes || echo no)" "yes"
155155
check "SH_MAX_COMMITTED_MB is non-zero" "$([ -n "$sh_max_committed" ] && [ "$sh_max_committed" -gt 0 ] && echo yes || echo no)" "yes"
156156

157+
echo "== the admission budget is checked against the host's physical memory, not just against itself"
158+
# SH_MAX_COMMITTED_MB less SH_MEMORY_RESERVE_MB is what the pool will admit, and nothing
159+
# in the worker reads MemTotal (grep -rE 'MemTotal|MemAvailable|meminfo|Sysinfo' over
160+
# remote-worker returns nothing). On a host with less RAM than the budget the gate
161+
# therefore cannot refuse before the OOM killer arrives, and spec §7.4's prediction 1
162+
# reads as confirmed by a gate that never refuses at all. AssertMemory is the control
163+
# that makes the budget a statement about THIS host: systemd compares it against
164+
# physical memory and fails the unit at start, the same posture as
165+
# AssertPathExists=/dev/kvm.
166+
assert_memory_raw="$(sed -n '/^\[Unit\]/,/^\[Service\]/p' "$SERVICE" | grep -oE '^AssertMemory=[<>=]*[0-9]+[KMGT]?' | head -n1 | cut -d= -f2-)"
167+
check "AssertMemory is set in [Unit]" \
168+
"$([ -n "$assert_memory_raw" ] && echo yes || echo no)" "yes"
169+
# The operator must be >= : "at least this much RAM". An = or <= would assert something
170+
# else entirely (and a bare size means =, which would refuse on any larger host).
171+
check "AssertMemory uses the >= comparison (at LEAST this much physical memory)" \
172+
"$(printf '%s' "$assert_memory_raw" | grep -cE '^>=[0-9]+[KMGT]?$')" "1"
173+
# Normalised to MiB so it can be compared with SH_MAX_COMMITTED_MB, which is in MiB.
174+
# systemd's size suffixes are 1024-based.
175+
assert_memory_mb=""
176+
case "$assert_memory_raw" in
177+
*T) assert_memory_mb=$(( $(printf '%s' "$assert_memory_raw" | tr -dc '0-9') * 1024 * 1024 )) ;;
178+
*G) assert_memory_mb=$(( $(printf '%s' "$assert_memory_raw" | tr -dc '0-9') * 1024 )) ;;
179+
*M) assert_memory_mb=$(printf '%s' "$assert_memory_raw" | tr -dc '0-9') ;;
180+
*K) assert_memory_mb=$(( $(printf '%s' "$assert_memory_raw" | tr -dc '0-9') / 1024 )) ;;
181+
*[0-9]) assert_memory_mb=$(( $(printf '%s' "$assert_memory_raw" | tr -dc '0-9') / 1024 / 1024 )) ;;
182+
esac
183+
# Two bounds, and both are load-bearing:
184+
#
185+
# - not far BELOW the budget: raising SH_MAX_COMMITTED_MB without raising the
186+
# assertion is exactly the drift that leaves a 16 GiB host admitting 20 GiB of VMs.
187+
# The 90% floor is the tolerance the assertion needs, because physical memory here is
188+
# MemTotal, which is always somewhat below nominal RAM.
189+
# - not ABOVE the budget: an assertion at or above nominal RAM refuses to start on a
190+
# host of exactly the size the budget was written for, because of that same MemTotal
191+
# shortfall. That would be a self-inflicted outage, so it is a test failure too.
192+
budget_floor=$(( sh_max_committed * 90 / 100 ))
193+
check "AssertMemory is within the tolerance band of SH_MAX_COMMITTED_MB (>= 90% of it, and not above it)" \
194+
"$([ -n "$assert_memory_mb" ] && [ "$assert_memory_mb" -ge "$budget_floor" ] && [ "$assert_memory_mb" -le "$sh_max_committed" ] && echo yes || echo no)" "yes"
195+
# The reserve is headroom INSIDE the ceiling (budget.go: MaxCommittedBytes less
196+
# MemoryReserveBytes), so the ceiling itself is what the host has to be able to hold.
197+
sh_reserve="$(grep -oE 'SH_MEMORY_RESERVE_MB=[0-9]+' "$SERVICE" | head -n1 | cut -d= -f2)"
198+
check "SH_MEMORY_RESERVE_MB is set and smaller than SH_MAX_COMMITTED_MB (Config.Normalize refuses otherwise)" \
199+
"$([ -n "$sh_reserve" ] && [ "$sh_reserve" -lt "$sh_max_committed" ] && echo yes || echo no)" "yes"
200+
157201
echo "== the slice's swap posture (spec §6 mitigation #3: fast, attributable in-cgroup OOM kill)"
158202
check "MemorySwapMax=0 is set on the slice" \
159203
"$([ "$(grep -cF 'MemorySwapMax=0' "$SLICE")" -ge 1 ] && echo yes || echo no)" "yes"

0 commit comments

Comments
 (0)