Skip to content

Commit

Permalink
Fix integer overflows in btop_collect.cpp
Browse files Browse the repository at this point in the history
Correct data types in calls to std::accumulate(). The "bandwidth" deques
have type "long long", so the initial value of the accumulator (0) must
also be "long long" (i.e., "0ll") to prevent integer overflows. Also,
since since the bandwidth deques are (signed) "long long", the avg_speed
should presumably be a signed "long long" instead of an unsigned
"uint64_t". The previous behavior was for large bandwidth values to
overflow the accumulator, resulting in a negative total, which then was
cast to be a huge "uint64_t" value. As a consequence, the network graph
autoscaling was broken for large bandwidths.
  • Loading branch information
dorrellmw committed May 26, 2023
1 parent 808b8c1 commit 3c6929b
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/freebsd/btop_collect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -986,8 +986,8 @@ namespace Net {
for (const auto &dir : {"download", "upload"}) {
for (const auto &sel : {0, 1}) {
if (rescale or max_count[dir][sel] >= 5) {
const uint64_t avg_speed = (net[selected_iface].bandwidth[dir].size() > 5
? std::accumulate(net.at(selected_iface).bandwidth.at(dir).rbegin(), net.at(selected_iface).bandwidth.at(dir).rbegin() + 5, 0) / 5
const long long avg_speed = (net[selected_iface].bandwidth[dir].size() > 5
? std::accumulate(net.at(selected_iface).bandwidth.at(dir).rbegin(), net.at(selected_iface).bandwidth.at(dir).rbegin() + 5, 0ll) / 5
: net[selected_iface].stat[dir].speed);
graph_max[dir] = max(uint64_t(avg_speed * (sel == 0 ? 1.3 : 3.0)), (uint64_t)10 << 10);
max_count[dir][0] = max_count[dir][1] = 0;
Expand Down
6 changes: 3 additions & 3 deletions src/linux/btop_collect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ namespace Cpu {
if (times.size() < 4) throw std::runtime_error("Malformatted /proc/stat");

//? Subtract fields 8-9 and any future unknown fields
const long long totals = max(0ll, total_sum - (times.size() > 8 ? std::accumulate(times.begin() + 8, times.end(), 0) : 0));
const long long totals = max(0ll, total_sum - (times.size() > 8 ? std::accumulate(times.begin() + 8, times.end(), 0ll) : 0));

//? Add iowait field if present
const long long idles = max(0ll, times.at(3) + (times.size() > 4 ? times.at(4) : 0));
Expand Down Expand Up @@ -1543,8 +1543,8 @@ namespace Net {
for (const auto& dir: {"download", "upload"}) {
for (const auto& sel : {0, 1}) {
if (rescale or max_count[dir][sel] >= 5) {
const uint64_t avg_speed = (net[selected_iface].bandwidth[dir].size() > 5
? std::accumulate(net.at(selected_iface).bandwidth.at(dir).rbegin(), net.at(selected_iface).bandwidth.at(dir).rbegin() + 5, 0) / 5
const long long avg_speed = (net[selected_iface].bandwidth[dir].size() > 5
? std::accumulate(net.at(selected_iface).bandwidth.at(dir).rbegin(), net.at(selected_iface).bandwidth.at(dir).rbegin() + 5, 0ll) / 5
: net[selected_iface].stat[dir].speed);
graph_max[dir] = max(uint64_t(avg_speed * (sel == 0 ? 1.3 : 3.0)), (uint64_t)10 << 10);
max_count[dir][0] = max_count[dir][1] = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/osx/btop_collect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1026,8 +1026,8 @@ namespace Net {
for (const auto &dir : {"download", "upload"}) {
for (const auto &sel : {0, 1}) {
if (rescale or max_count[dir][sel] >= 5) {
const uint64_t avg_speed = (net[selected_iface].bandwidth[dir].size() > 5
? std::accumulate(net.at(selected_iface).bandwidth.at(dir).rbegin(), net.at(selected_iface).bandwidth.at(dir).rbegin() + 5, 0) / 5
const long long avg_speed = (net[selected_iface].bandwidth[dir].size() > 5
? std::accumulate(net.at(selected_iface).bandwidth.at(dir).rbegin(), net.at(selected_iface).bandwidth.at(dir).rbegin() + 5, 0ll) / 5
: net[selected_iface].stat[dir].speed);
graph_max[dir] = max(uint64_t(avg_speed * (sel == 0 ? 1.3 : 3.0)), (uint64_t)10 << 10);
max_count[dir][0] = max_count[dir][1] = 0;
Expand Down

0 comments on commit 3c6929b

Please sign in to comment.