[GH-ISSUE #4365] [Bug]: byte_size parsing accepts non-finite values ("inf"/"nan") triggering UB in the double-to-uint64_t cast, and "infinite" does not round-trip through byte_size_to_string #1094

New issue

Open

opened 2026-10-29 21:22:49 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-29 21:22:49 +00:00

Owner

Originally created by @xiaodu55 on GitHub (Sep 28, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4365

Bug Report

Version: current main (d9152a47, 2026-09-28), mooncake-store/include/common/byte_size.h

Environment: Ubuntu 24.04 x86_64, g++ 13.3, UBSan (-fsanitize=undefined,float-cast-overflow). The wrong results are platform-dependent (see the aarch64 note below).

Summary

try_string_to_byte_size() parses the numeric part with std::stod(), which happily parses the non-finite literals "inf", "infinity" and "nan". The result is only guarded by value < 0 and then converted with static_cast<uint64_t>(value). Casting a non-finite double to an integer type is undefined behavior (C++17 [conv.fpint]), and the observed result is platform-dependent:

x86-64 (g++ 13.3, UBSan): try_string_to_byte_size("inf") returns 0 (UBSan reports byte_size.h:67: runtime error: inf is outside the range of representable values of type 'long unsigned int'), and try_string_to_byte_size("nan") returns 9223372036854775808 (2^63).
On aarch64 the equivalent hardware conversion saturates instead (inf → UINT64_MAX, NaN → 0), so the same config string yields different garbage on the arm64 wheels vs the x86_64 ones.

Why it matters (callers)

All four call sites feed user-controlled configuration strings through this helper:

mooncake-store/src/real_client_main.cpp:134-135 — CLI flags --global_segment_size / --local_buffer_size. --global_segment_size=inf silently becomes 0 on x86_64 instead of being rejected (and UINT64_MAX on arm64).
mooncake-store/src/config/mmap_arena_config.cpp:63 — MC_MMAP_ARENA_POOL_SIZE: "nan" parses to 2^63, passes the parsed > 0 check, and sets a bogus ~8 EiB pool size instead of falling back to the default with a warning.
mooncake-store/src/config/hugepage_config.cpp:40 — MC_STORE_HUGEPAGE_SIZE.
mooncake-store/src/real_client.cpp:1357 — env-based parsing.

The established contract for invalid input in this helper is "return std::nullopt" (that is what -5, 16XB, etc. do). Non-finite values bypass that contract and additionally invoke UB.

Second, related inconsistency: try_string_to_byte_size("infinite") returns UINT64_MAX, but byte_size_to_string() only prints "infinite" for INT64_MAX, so the parse → print round trip yields "16777216.00 TB" instead of "infinite". INT64_MAX is used as an "infinite" sentinel in several places (e.g. allocator.cpp, master_service.cpp), so byte_size_to_string() should recognize both sentinels.

Minimal repro

#include "common/byte_size.h"
#include <cstdio>

using namespace mooncake;

int main() {
    auto parsed = try_string_to_byte_size("infinite");
    std::printf("to_string(parse(\"infinite\")) = \"%s\"  (expected: infinite)\n",
                byte_size_to_string(parsed.value()).c_str());
    for (const char* in : {"inf", "infinity", "nan", "1e999"}) {
        auto r = try_string_to_byte_size(in);
        std::printf("parse(\"%s\").has_value() = %d", in, r.has_value());
        if (r.has_value()) std::printf(", value = %llu", (unsigned long long)r.value());
        std::printf("\n");
    }
}

Output (x86_64, g++ 13.3, -fsanitize=undefined,float-cast-overflow):

common/byte_size.h:67:38: runtime error: inf is outside the range of representable values of type 'long unsigned int'
to_string(parse("infinite")) = "16777216.00 TB"  (expected: infinite)
parse("inf").has_value() = 1, value = 0
parse("infinity").has_value() = 1, value = 0
parse("nan").has_value() = 1, value = 9223372036854775808
parse("1e999").has_value() = 0

Proposed fix

try_string_to_byte_size(): reject non-finite parsed values (!std::isfinite(value) → std::nullopt), keeping the existing "nullopt on invalid input" contract.
byte_size_to_string(): print "infinite" for both UINT64_MAX and INT64_MAX, without the implementation-defined int64_t round-trip cast.
Add tests for inf / infinity / nan / 1e999 and the infinite round trip in byte_size_test.cpp.

I have the fix and tests ready locally and will submit a PR right away.

Before submitting...

Ensure you searched for relevant issues and read the documentation

Originally created by @xiaodu55 on GitHub (Sep 28, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4365 ### Bug Report **Version:** current `main` (`d9152a47`, 2026-09-28), `mooncake-store/include/common/byte_size.h` **Environment:** Ubuntu 24.04 x86_64, g++ 13.3, UBSan (`-fsanitize=undefined,float-cast-overflow`). The wrong results are platform-dependent (see the aarch64 note below). **Summary** `try_string_to_byte_size()` parses the numeric part with `std::stod()`, which happily parses the non-finite literals `"inf"`, `"infinity"` and `"nan"`. The result is only guarded by `value < 0` and then converted with `static_cast<uint64_t>(value)`. Casting a non-finite `double` to an integer type is undefined behavior (C++17 [conv.fpint]), and the observed result is platform-dependent: - x86-64 (g++ 13.3, UBSan): `try_string_to_byte_size("inf")` returns **0** (UBSan reports `byte_size.h:67: runtime error: inf is outside the range of representable values of type 'long unsigned int'`), and `try_string_to_byte_size("nan")` returns **9223372036854775808** (2^63). - On aarch64 the equivalent hardware conversion saturates instead (inf → UINT64_MAX, NaN → 0), so the same config string yields different garbage on the arm64 wheels vs the x86_64 ones. **Why it matters (callers)** All four call sites feed user-controlled configuration strings through this helper: 1. `mooncake-store/src/real_client_main.cpp:134-135` — CLI flags `--global_segment_size` / `--local_buffer_size`. `--global_segment_size=inf` silently becomes 0 on x86_64 instead of being rejected (and UINT64_MAX on arm64). 2. `mooncake-store/src/config/mmap_arena_config.cpp:63` — `MC_MMAP_ARENA_POOL_SIZE`: `"nan"` parses to 2^63, passes the `parsed > 0` check, and sets a bogus ~8 EiB pool size instead of falling back to the default with a warning. 3. `mooncake-store/src/config/hugepage_config.cpp:40` — `MC_STORE_HUGEPAGE_SIZE`. 4. `mooncake-store/src/real_client.cpp:1357` — env-based parsing. The established contract for invalid input in this helper is "return `std::nullopt`" (that is what `-5`, `16XB`, etc. do). Non-finite values bypass that contract and additionally invoke UB. **Second, related inconsistency:** `try_string_to_byte_size("infinite")` returns `UINT64_MAX`, but `byte_size_to_string()` only prints `"infinite"` for `INT64_MAX`, so the parse → print round trip yields `"16777216.00 TB"` instead of `"infinite"`. `INT64_MAX` is used as an "infinite" sentinel in several places (e.g. `allocator.cpp`, `master_service.cpp`), so `byte_size_to_string()` should recognize both sentinels. **Minimal repro** ```cpp #include "common/byte_size.h" #include <cstdio> using namespace mooncake; int main() { auto parsed = try_string_to_byte_size("infinite"); std::printf("to_string(parse(\"infinite\")) = \"%s\" (expected: infinite)\n", byte_size_to_string(parsed.value()).c_str()); for (const char* in : {"inf", "infinity", "nan", "1e999"}) { auto r = try_string_to_byte_size(in); std::printf("parse(\"%s\").has_value() = %d", in, r.has_value()); if (r.has_value()) std::printf(", value = %llu", (unsigned long long)r.value()); std::printf("\n"); } } ``` **Output (x86_64, g++ 13.3, `-fsanitize=undefined,float-cast-overflow`):** ```text common/byte_size.h:67:38: runtime error: inf is outside the range of representable values of type 'long unsigned int' to_string(parse("infinite")) = "16777216.00 TB" (expected: infinite) parse("inf").has_value() = 1, value = 0 parse("infinity").has_value() = 1, value = 0 parse("nan").has_value() = 1, value = 9223372036854775808 parse("1e999").has_value() = 0 ``` **Proposed fix** 1. `try_string_to_byte_size()`: reject non-finite parsed values (`!std::isfinite(value)` → `std::nullopt`), keeping the existing "nullopt on invalid input" contract. 2. `byte_size_to_string()`: print `"infinite"` for both `UINT64_MAX` and `INT64_MAX`, without the implementation-defined `int64_t` round-trip cast. 3. Add tests for `inf` / `infinity` / `nan` / `1e999` and the `infinite` round trip in `byte_size_test.cpp`. I have the fix and tests ready locally and will submit a PR right away. ### Before submitting... - [x] Ensure you searched for relevant issues and read the [documentation](https://kvcache-ai.github.io/Mooncake/)

bsakaguchi commented

2026-10-29 21:22:50 +00:00

Author

Owner

@xiaodu55 commented on GitHub (Sep 28, 2026):

I'll take this one — fix and tests are ready locally; PR to follow shortly.

@xiaodu55 commented on GitHub (Sep 28, 2026): I'll take this one — fix and tests are ready locally; PR to follow shortly.

bsakaguchi commented

2026-10-29 21:22:50 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Sep 28, 2026):

Thanks for opening this issue, @xiaodu55!

Field	Value
Issue	#4365
GitHub user ID	`203964531`
Reporter	@xiaodu55

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Sep 28, 2026): Thanks for opening this issue, @xiaodu55! | Field | Value | |-------|-------| | **Issue** | #4365 | | **GitHub user ID** | `203964531` | | **Reporter** | @xiaodu55 | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi referenced this issue

2026-10-29 21:32:33 +00:00

[PR #1205] [MERGED] [TE] Memorize batched transfer status #2070