bottlerocket-os / bottlerocket-os/bottlerocket-settings-sdk

Static hugepages accepts two spellings of the same page size

Open
#152 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
5
Forks
36
Avg merge
4d 12h
Merged PRs (30d)
2

Description

## What I expected to happen

`settings.kernel.hugepages.static` holds one pool per page size, so I
expected it to be impossible to configure the same pool twice.

## What actually happened

`HugepageSize` keeps the string it was given, so `2Mi` and `2048Ki` are
distinct keys in `hugepages_config` even though `as_kib()` returns 2048
for both:

```rust
let a = HugepageSize::try_from("2Mi").unwrap();
let b = HugepageSize::try_from("2048Ki").unwrap();
assert_eq!(a.as_kib(), b.as_kib()); // both Some(2048)
assert_ne!(a, b); // but distinct map keys
```

So this deserializes fine today, with two counts for one pool:

```toml
[settings.kernel.hugepages.static]
"2Mi" = { count = "512" }
"2048Ki" = { count = "4" }
```

## Why it matters

`corndog` addresses a pool by its size in kibibytes
([hugepages.rs](https://github.com/bottlerocket-os/bottlerocket-core-kit/blob/develop/sources/api/corndog/src/hugepages.rs)):

```rust
let size_dir_name = format!("hugepages-{page_size}kB");
```

Both entries therefore resolve to
`/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages`.
`set_static_hugepages` sorts the allocations by
`Reverse(size.as_kib())`, which is the same key for both, so their
relative order is whatever the `HashMap` iteration produced. Both get
written and the last one wins, which means the number of pages the
instance ends up with is not stable across boots for an unchanged
config.

`essential = true` does not catch it either: each write is verified by
reading the value back, and the second write's read-back matches its own
request, so the shortfall check passes while the first request is
silently discarded.

The same applies to any equivalent pair, e.g. `1Gi` / `1024Mi`.

## Suggested fix

Reject the input while deserializing, rather than silently keeping one
of the two counts. I have a small patch that does this and will open a
PR shortly; happy to switch to normalizing the key instead if you would
rather have equivalent spellings collapse into one pool, though that
makes which count survives an implementation detail.

Contributor guide

Open the contributing guide

Research direction

Start with hugepages.rs and trace HugepageSize deserialization alongside as_kib(), then review how static hugepage entries become hugepages_config keys. The work is done when equivalent spellings such as 2Mi and 2048Ki are rejected during deserialization rather than accepted as separate pools.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
operating-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.