Skip to content

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

Description

@thc1006

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    llm-generatedUsed to tag LLM-generated issues or PRs, which some maintainers may choose to de-prioritise.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions