Skip to content

Comments

pkg/sysinfo: internalize parsing cpusets#49193

Merged
thaJeztah merged 5 commits intomoby:masterfrom
thaJeztah:internalize_pkg_parsers
Jan 6, 2025
Merged

pkg/sysinfo: internalize parsing cpusets#49193
thaJeztah merged 5 commits intomoby:masterfrom
thaJeztah:internalize_pkg_parsers

Conversation

@thaJeztah
Copy link
Member


  • pkg/sysinfo: stub out parsing cpusets on non-linux
  • pkg/sysinfo: rename vars/arguments for clarity
  • pkg/sysinfo: define const for default Max CPUs
  • pkg/sysinfo: touch-up docs for cgroupCpusetInfo.Cpus, Mems
  • pkg/sysinfo: internalize parsing cpusets

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code area/go-sdk labels Jan 1, 2025
@thaJeztah thaJeztah added this to the 28.0.0 milestone Jan 1, 2025
@thaJeztah thaJeztah self-assigned this Jan 1, 2025
return false, err
}
// Start with the normal maximum number of CPUs on Linux, but accept
// more if we actually have more CPUs available.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of having this maxCPUs variable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to dig in history a bit, but It was added in f8e876d;

Related CVE was CVE-2018-20699, with some discussion here; docker-archive#70 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me add that to the comment

@thaJeztah thaJeztah force-pushed the internalize_pkg_parsers branch from e3653f9 to 2282279 Compare January 6, 2025 09:46
Comment on lines +329 to +338
//
// This limit was added in f8e876d7616469d07b8b049ecb48967eeb8fa7a5
// to address CVE-2018-20699:
//
// Using a value such as `--cpuset-mems=1-9223372036854775807` would cause
// dockerd to run out of memory allocating a map of the values in the
// validation code. Set limits to the normal limit of the number of CPUs.
//
// More details in https://github.com/docker-archive/engine/pull/70#issuecomment-458458288
maxCPUs := defaultMaxCPUs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment here to link to the relevant information

@thaJeztah thaJeztah merged commit 120f616 into moby:master Jan 6, 2025
139 checks passed
@thaJeztah thaJeztah deleted the internalize_pkg_parsers branch January 6, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/go-sdk kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants