opencontainers / opencontainers/cgroups
systemd: addCPUQuota overflows int64 for a large CPU quota, writing a tiny value
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 29
- Forks
- 32
- PR merge metrics
- No merged PRs in 30d
Description
addCPUQuota in systemd/common.go turns a container's CFS quota into systemd's CPUQuotaPerSecUSec and multiplies the quota by 1,000,000 in signed 64-bit arithmetic. Once the quota is large enough that multiply overflows and wraps, so a container that asked for an effectively unlimited quota gets a tiny or invalid one instead.
Where it happens
The two conversions in systemd/common.go:
cpuQuotaPerSecUSec = uint64(*quota*1000000) / period // (1)
if cpuQuotaPerSecUSec%10000 != 0 {
cpuQuotaPerSecUSec = ((cpuQuotaPerSecUSec / 10000) + 1) * 10000
*quota = int64(cpuQuotaPerSecUSec) * int64(period) / 1000000 // (2)
}
*quota * 1000000 in (1) is an int64 multiply, so it wraps once *quota passes math.MaxInt64 / 1000000, about 9.2e12. (2) has the same shape and can wrap on the round-up even when (1) does not, since int64(cpuQuotaPerSecUSec) * int64(period) grows past MaxInt64 for a large rounded value.
Reproduction
Take a quota near the largest a caller might pass, with the default 100ms period:
quota = 9223372036854700 // finite, below math.MaxInt64
period = 100000
(1) uint64(quota*1000000) / period = 184467440736337 // quota*1000000 overflowed int64 before the cast
(2) rounded value written back to quota = 290
The quota is rewritten from about 9.2e15 down to 290. The kernel's min_cfs_quota_period is 1ms, so a positive cpu.cfs_quota_us below 1000 is rejected (see CFS bandwidth control). A container that asked for a huge quota therefore fails the cgroupfs write rather than running effectively unthrottled.
Why it matters
A quota this large only comes from an enormous CPU limit, so it is not an everyday value, but it is a valid one and today it fails in a confusing way. The write-back at (2) landed in #4 (a carry of runc#4639) to keep cgroupfs and the systemd property in agreement, which means the wrapped value now reaches cgroupfs as well, not only systemd.
Possible directions
I would rather match your intent for the round-up before sending a patch, so two options:
- Treat a quota too large to convert as effectively unlimited, the same way
*quota <= 0already maps toUSEC_INFINITY. Small change, but it turns a finite quota into an unlimited one past the boundary. - Do (1) and (2) in 128-bit with
math/bits.Mul64andDiv64, falling back to unlimited only when the result cannot be represented. Keeps the value exact where it fits, at the cost of a little more code.
Happy to send the PR either way once you point me at the one you prefer.
Note
Found while hardening the kubelet-side conversion in kubernetes/kubernetes#141327. That change only maps values whose local milliCPU * period product overflows int64 to the no-quota sentinel, so finite quotas below that boundary (including the reproducer above) still reach and overflow addCPUQuota independently.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in systemd/common.go at the addCPUQuota conversions around lines 324-328, and review the existing USEC_INFINITY handling. Confirm the overflow with the quota and period from the reproduction, then agree on the intended finite-versus-unlimited behavior before adding regression coverage. Done means large valid quotas no longer become tiny or invalid values when written to systemd or cgroupfs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100