Skip to content

Commit c1cc058

Browse files
committed
fix: compute jailer cpu time without underflow
The jailer wishes to compute the CPU time it spents before exec-ing into Firecracker, so that it can be passed to Firecracker's `parent-cpu-us` flag and emitted as a metric. The CLOCK_PROCESS_CPUTIME_ID is reset on every fork() call, so we need to stitch together the different values from before the first fork, and then between each consecutive fork. However, we also need to account for the scenarios that we do not fork() at all (no daemonization). Now, this all goes slightly wrong because we want to exclude the time spent the call to Env::run() from the reported time (note that inside the jailer, this is only the time spent parsing arguments, but if a user ended up exec()-ing into the jailer, it would also include the time from prior to calling exec(), which makes sense to exclude). So we grab the value of CLOCK_PROCESS_CPUTIME_ID right after parsing arguments, and subtract it at the end. Sadly, in the case of daemonization, we subtract it not from the total amount of time spent in the jailer, but rather from the time spent since the final fork(). This works out if the time spent since the last fork was greater than the time spent parsing arguments, but if not, we have an integer underflow on our hands. Now, this is only a problem in debug builds, and in release builds, we underflow, and then _over_flow again during the next addition of the underflowed value to jailer_cpu_time_us, and Firecracker even seens a sane value for parent_time_cpu_us. But in debug builds, we panic. Fix this by breaking the subtraction and addition into two separate statements, to avoid relying on the double over-/underflow Closes #5033 Fixes: 68ab56e Signed-off-by: Patrick Roy <[email protected]>
1 parent 9bd3bb9 commit c1cc058

File tree

2 files changed

+7
-1
lines changed

2 files changed

+7
-1
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ and this project adheres to
4949
balloon device is inflated post UFFD-backed snapshot restore, Firecracker now
5050
causes `remove` UFFD messages to be sent to the UFFD handler. Previously, no
5151
such message would be sent.
52+
- [#5034](https://github.com/firecracker-microvm/firecracker/pull/5034): Fix an
53+
integer underflow in the jailer when computing the value it passes to
54+
Firecracker's `--parent-cpu-time-us` values, which caused development builds
55+
of Firecracker to crash (but production builds were unaffected as underflows
56+
do not panic in release mode).
5257

5358
## [1.10.1]
5459

src/jailer/src/env.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,8 @@ impl Env {
696696
}
697697

698698
// Compute jailer's total CPU time up to the current time.
699-
self.jailer_cpu_time_us += get_time_us(ClockType::ProcessCpu) - self.start_time_cpu_us;
699+
self.jailer_cpu_time_us += get_time_us(ClockType::ProcessCpu);
700+
self.jailer_cpu_time_us -= self.start_time_cpu_us;
700701
// Reset process start time.
701702
self.start_time_cpu_us = 0;
702703

0 commit comments

Comments
 (0)