What
LxcStateAwareBackend::start decides what to do from container.is_running()
and then acts on that decision, with nothing holding the container still in
between.
src/backends/lxc/common/src/state_aware.rs:498-520:
if container.is_running() {
if has_filesystem_policy(..) || has_network_policy(..) {
return Err(MxcError::already_started(..));
}
} else {
apply_filesystem_policy(..)?;
// install firewall, then start
}Two start calls for the same sandbox id can both observe is_running() == false and both take the else arm. Both then apply filesystem policy and
install firewall state for the same container.
Why it matters
The already_started guard exists to stop a second caller reapplying start
policy to a live container. A race defeats exactly that guard, and it does so
silently: the second caller gets Ok, not the error the guard was written to
produce.
Fix
Serialize the phase per sandbox id -- an advisory lock on a per-sandbox file
(flock on Linux) taken before the is_running() probe and held across the
start, so the check and the act are one critical section. Roughly 20-40 lines.
Provenance
Reported by Copilot on PR #849. Held out of that PR deliberately: it is a
design change to the concurrency model rather than a fix to the code under
review, and it deserves its own review.
Severity: low. The one-shot CLI path constructs one backend per invocation, so
this needs two concurrent processes driving the same sandbox id to reach.
What
LxcStateAwareBackend::startdecides what to do fromcontainer.is_running()and then acts on that decision, with nothing holding the container still in
between.
src/backends/lxc/common/src/state_aware.rs:498-520:Two
startcalls for the same sandbox id can both observeis_running() == falseand both take theelsearm. Both then apply filesystem policy andinstall firewall state for the same container.
Why it matters
The
already_startedguard exists to stop a second caller reapplying startpolicy to a live container. A race defeats exactly that guard, and it does so
silently: the second caller gets
Ok, not the error the guard was written toproduce.
Fix
Serialize the phase per sandbox id -- an advisory lock on a per-sandbox file
(
flockon Linux) taken before theis_running()probe and held across thestart, so the check and the act are one critical section. Roughly 20-40 lines.
Provenance
Reported by Copilot on PR #849. Held out of that PR deliberately: it is a
design change to the concurrency model rather than a fix to the code under
review, and it deserves its own review.
Severity: low. The one-shot CLI path constructs one backend per invocation, so
this needs two concurrent processes driving the same sandbox id to reach.