[GH-ISSUE #2315] [Bug]: Environ::GetInt uses atoi with no error detection, opendir() leaks DIR* handle #633

New issue

Closed

opened 2026-10-29 21:17:26 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 21:17:26 +00:00

Owner

Originally created by @jfeng18 on GitHub (Jun 3, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2315

Description

1. Environ::GetInt uses std::atoi with no error detection

Environ::GetInt() in mooncake-common/src/environ.cpp uses std::atoi() which returns 0 on parse failure and has undefined behavior on overflow. Setting MC_MAX_WR=abc silently yields 0 instead of the intended default (256). All 35+ MC_* environment variables parsed through GetInt/GetSizeT are affected.

Additionally, GetSizeT uses strtoull which silently wraps negative values: MC_SLICE_SIZE=-1 would set slice_size to ~18 exabytes (ULLONG_MAX) instead of the default.

2. opendir() handle leaked in loadGlobalConfig()

config.cpp line 273 calls opendir(log_dir_path) to check if a directory exists, but the returned DIR* is never closedir()'d, leaking one file descriptor per process.

3. .yml extension not recognized by DefaultConfig::Load()

DefaultConfig::Load() only recognizes .yaml but not .yml, which is equally common.

Impact

Silent misconfiguration: invalid env var values parsed as 0 instead of using defaults
File descriptor leak (one per process, on startup)
.yml config files rejected with "Unsupported config file format"

Environment

All platforms. The Environ singleton is used across the entire codebase.

Originally created by @jfeng18 on GitHub (Jun 3, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2315 ### Description **1. `Environ::GetInt` uses `std::atoi` with no error detection** `Environ::GetInt()` in `mooncake-common/src/environ.cpp` uses `std::atoi()` which returns 0 on parse failure and has undefined behavior on overflow. Setting `MC_MAX_WR=abc` silently yields 0 instead of the intended default (256). All 35+ `MC_*` environment variables parsed through `GetInt`/`GetSizeT` are affected. Additionally, `GetSizeT` uses `strtoull` which silently wraps negative values: `MC_SLICE_SIZE=-1` would set slice_size to ~18 exabytes (ULLONG_MAX) instead of the default. **2. `opendir()` handle leaked in `loadGlobalConfig()`** `config.cpp` line 273 calls `opendir(log_dir_path)` to check if a directory exists, but the returned `DIR*` is never `closedir()`'d, leaking one file descriptor per process. **3. `.yml` extension not recognized by `DefaultConfig::Load()`** `DefaultConfig::Load()` only recognizes `.yaml` but not `.yml`, which is equally common. ### Impact - Silent misconfiguration: invalid env var values parsed as 0 instead of using defaults - File descriptor leak (one per process, on startup) - `.yml` config files rejected with "Unsupported config file format" ### Environment All platforms. The `Environ` singleton is used across the entire codebase.

bsakaguchi closed this issue

2026-10-29 21:17:27 +00:00

bsakaguchi referenced this issue

2026-10-29 21:29:39 +00:00

[PR #633] [CLOSED] [do not merge]: add g++ flags for (#571) to allow proper arm build #1704