opencontainers / opencontainers/cgroups

systemd: addCPUQuota overflows int64 for a large CPU quota, writing a tiny value

Open
#73 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

llm-generated
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:

  1. Treat a quota too large to convert as effectively unlimited, the same way *quota <= 0 already maps to USEC_INFINITY. Small change, but it turns a finite quota into an unlimited one past the boundary.
  2. Do (1) and (2) in 128-bit with math/bits.Mul64 and Div64, 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.