brbzull0 opened a new pull request, #13905: URL: https://github.com/apache/trafficserver/pull/13905
`ink_get_current_rss()`, added in #13219, multiplies the resident page count read from `/proc/self/statm` by the page size with no bound, and `MemoryLimit::periodic()` casts the result to `int64_t` for the `proxy.process.traffic_server.memory.rss` gauge. Coverity treats the page count as tainted input, so it reports the multiply as a possible overflow and the cast as a possible negative value. A real resident set is far below the `int64_t` range, so this is hardening, not a fix for an observed failure. Coverity CIDs: [1660035](https://scan6.scan.coverity.com/#/project-view/58063/10191?selectedIssue=1660035), [1660039](https://scan6.scan.coverity.com/#/project-view/58063/10191?selectedIssue=1660039) **What changes:** - A new helper, `ink_rss_pages_to_bytes()` in `include/tscore/ink_sys_control.h`, compares the page count with `INT64_MAX / page_size` before it multiplies. It returns 0 when the product would not fit an `int64_t` or the page size is 0. 0 is what `ink_get_current_rss()` already documents for "could not be determined", and what its error paths return. The header is internal (`include/tscore` is not installed); there is no TS API change. - The Linux and FreeBSD branches of `ink_get_current_rss()` use the helper. FreeBSD also returns 0 for a negative `ki_rssize`, which is a signed `segsz_t`. The macOS branch, which reads bytes directly, applies the same bound. - `MemoryLimit::periodic()` clamps the reading before the signed cast, so the gauge stays non-negative whatever `ink_get_current_rss()` returns. - An out-of-range reading is reported as 0 rather than saturated to the largest value: a saturated reading would turn the accept throttle on and refuse new connections on a value no process can have. - Tests: - `src/tscore/unit_tests/test_ink_sys_control.cc` covers the helper: ordinary products, a page size of 0, the largest count that fits and one page past it at several page sizes, and counts whose product would wrap, including ones that wrap to a small value that a check made after the multiply would accept. - `test_memory_limit_applies` in `tests/uranium_tests/client_connection/test_connections_throttle_exempt.py` now also reads `proxy.process.traffic_server.memory.rss` once the throttle is on, and checks that it is a plausible byte count. **What this PR does not do:** - No test feeds an out-of-range reading through `ink_get_current_rss()` itself, or reaches the clamp in `MemoryLimit::periodic()`: the function reads `/proc/self/statm`, `sysctl()` and `task_info()` directly, with no seam a unit test can replace. The helper tests are what pin the arithmetic. - It does not change what a failed read returns. A failed `fopen()` or `fscanf()` still reports 0, which releases the throttle for that sample. - The 10.2.x branch has the same code, from the cherry-pick 3bcebe8fbf. This PR targets master and is marked for backport. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
