From 143147ebf15b2e00b24f53e9eb5024878dffcab3 Mon Sep 17 00:00:00 2001 From: Daniel Lange Date: Wed, 17 Jun 2026 03:42:19 +0200 Subject: [PATCH 1/3] Fix segfault when vCPUs are hot-added: resize AllCPUsMeter data->meters array on CPU count change When CPUs are hot-added while htop is running, LinuxMachine_scanCPUInfo updates existingCPUs. The next Header_updateData call invokes AllCPUsMeter_updateValues, which queries the new (larger) count from AllCPUsMeter_getRange but accesses data->meters that was sized for the old count hence causing an out-of-bounds access and SIGSEGV. Changes: - Rewriting CPUMeterCommonInit to detect count changes and resize data->meters (freeing excess meters when shrinking, reallocating and zero-filling when growing). data->cpus now stores the actual allocated meter count rather than existingCPUs. - Having AllCPUsMeter_updateValues detect a count mismatch and call CPUMeterCommonInit before iterating over the meters array. - Updating AllCPUsMeter_done to iterate over data->cpus (the number of meters actually allocated) instead of re-deriving via AllCPUsMeter_getRange, avoiding the same class of bug at teardown. - Having CPUMeterCommonUpdateMode call CPUMeterCommonInit upfront for the same reason. Fixes: #2024 Assisted-by: Microsoft Copilot/Claude Sonnet 4.6 --- CPUMeter.c | 51 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/CPUMeter.c b/CPUMeter.c index 7bc096dbc..b7272a4ec 100644 --- a/CPUMeter.c +++ b/CPUMeter.c @@ -252,15 +252,6 @@ static void AllCPUsMeter_getRange(const Meter* this, int* start, int* count) { } } -static void AllCPUsMeter_updateValues(Meter* this) { - CPUMeterData* data = this->meterData; - Meter** meters = data->meters; - int start, count; - AllCPUsMeter_getRange(this, &start, &count); - for (int i = 0; i < count; i++) - Meter_updateValues(meters[i]); -} - static void CPUMeterCommonInit(Meter* this) { int start, count; AllCPUsMeter_getRange(this, &start, &count); @@ -268,11 +259,30 @@ static void CPUMeterCommonInit(Meter* this) { CPUMeterData* data = this->meterData; if (!data) { data = xCalloc(1, sizeof(CPUMeterData)); - data->cpus = this->host->existingCPUs; - data->meters = count ? xCalloc(count, sizeof(Meter*)) : NULL; + data->cpus = 0; /* no meters allocated yet */ + data->meters = NULL; this->meterData = data; } + assert(count >= 0); + unsigned int prevCount = data->cpus; + if ((unsigned int)count != prevCount) { + /* free meters for CPUs that are no longer in range */ + for (unsigned int i = (unsigned int)count; i < prevCount; i++) + Meter_delete((Object*)data->meters[i]); + + if (count > 0) { + data->meters = xReallocArray(data->meters, (unsigned int)count, sizeof(Meter*)); + /* zero-fill newly added entries */ + if ((unsigned int)count > prevCount) + memset(data->meters + prevCount, 0, ((unsigned int)count - prevCount) * sizeof(Meter*)); + } else { + free(data->meters); + data->meters = NULL; + } + data->cpus = (unsigned int)count; + } + Meter** meters = data->meters; for (int i = 0; i < count; i++) { if (!meters[i]) @@ -282,7 +292,22 @@ static void CPUMeterCommonInit(Meter* this) { } } +static void AllCPUsMeter_updateValues(Meter* this) { + CPUMeterData* data = this->meterData; + int start, count; + AllCPUsMeter_getRange(this, &start, &count); + /* Reinit if the number of CPUs changed (e.g. hot-plug) */ + if ((unsigned int)count != data->cpus) + CPUMeterCommonInit(this); + data = this->meterData; + Meter** meters = data->meters; + for (int i = 0; i < count; i++) + Meter_updateValues(meters[i]); +} + static void CPUMeterCommonUpdateMode(Meter* this, MeterModeId mode, int ncol) { + /* Reinit first in case the CPU count changed since last init */ + CPUMeterCommonInit(this); CPUMeterData* data = this->meterData; Meter** meters = data->meters; this->mode = mode; @@ -303,9 +328,7 @@ static void CPUMeterCommonUpdateMode(Meter* this, MeterModeId mode, int ncol) { static void AllCPUsMeter_done(Meter* this) { CPUMeterData* data = this->meterData; Meter** meters = data->meters; - int start, count; - AllCPUsMeter_getRange(this, &start, &count); - for (int i = 0; i < count; i++) + for (unsigned int i = 0; i < data->cpus; i++) Meter_delete((Object*)meters[i]); free(data->meters); free(data); From 6f86c5f8e9b979dfb4f461ba174d25cb753e7b6c Mon Sep 17 00:00:00 2001 From: Daniel Lange Date: Wed, 17 Jun 2026 04:14:41 +0200 Subject: [PATCH 2/3] Housekeeping: Make count, start, ncol unsigned, removes lots of casting Assisted-by: Microsoft Copilot/Claude Sonnet 4.6 --- CPUMeter.c | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/CPUMeter.c b/CPUMeter.c index b7272a4ec..ed4fee4e2 100644 --- a/CPUMeter.c +++ b/CPUMeter.c @@ -233,7 +233,7 @@ static void CPUMeter_display(const Object* cast, RichString* out) { #endif } -static void AllCPUsMeter_getRange(const Meter* this, int* start, int* count) { +static void AllCPUsMeter_getRange(const Meter* this, unsigned int* start, unsigned int* count) { unsigned int cpus = this->host->existingCPUs; switch (Meter_name(this)[0]) { default: @@ -253,7 +253,7 @@ static void AllCPUsMeter_getRange(const Meter* this, int* start, int* count) { } static void CPUMeterCommonInit(Meter* this) { - int start, count; + unsigned int start, count, prevCount; AllCPUsMeter_getRange(this, &start, &count); CPUMeterData* data = this->meterData; @@ -264,27 +264,26 @@ static void CPUMeterCommonInit(Meter* this) { this->meterData = data; } - assert(count >= 0); - unsigned int prevCount = data->cpus; - if ((unsigned int)count != prevCount) { + prevCount = data->cpus; + if (count != prevCount) { /* free meters for CPUs that are no longer in range */ - for (unsigned int i = (unsigned int)count; i < prevCount; i++) + for (unsigned int i = count; i < prevCount; i++) Meter_delete((Object*)data->meters[i]); if (count > 0) { - data->meters = xReallocArray(data->meters, (unsigned int)count, sizeof(Meter*)); + data->meters = xReallocArray(data->meters, count, sizeof(Meter*)); /* zero-fill newly added entries */ - if ((unsigned int)count > prevCount) - memset(data->meters + prevCount, 0, ((unsigned int)count - prevCount) * sizeof(Meter*)); + if (count > prevCount) + memset(data->meters + prevCount, 0, (count - prevCount) * sizeof(Meter*)); } else { free(data->meters); data->meters = NULL; } - data->cpus = (unsigned int)count; + data->cpus = count; } Meter** meters = data->meters; - for (int i = 0; i < count; i++) { + for (unsigned int i = 0; i < count; i++) { if (!meters[i]) meters[i] = Meter_new(this->host, start + i + 1, (const MeterClass*) Class(CPUMeter)); @@ -294,30 +293,30 @@ static void CPUMeterCommonInit(Meter* this) { static void AllCPUsMeter_updateValues(Meter* this) { CPUMeterData* data = this->meterData; - int start, count; + unsigned int start, count; AllCPUsMeter_getRange(this, &start, &count); /* Reinit if the number of CPUs changed (e.g. hot-plug) */ - if ((unsigned int)count != data->cpus) + if (count != data->cpus) CPUMeterCommonInit(this); data = this->meterData; Meter** meters = data->meters; - for (int i = 0; i < count; i++) + for (unsigned int i = 0; i < count; i++) Meter_updateValues(meters[i]); } -static void CPUMeterCommonUpdateMode(Meter* this, MeterModeId mode, int ncol) { +static void CPUMeterCommonUpdateMode(Meter* this, MeterModeId mode, unsigned int ncol) { /* Reinit first in case the CPU count changed since last init */ CPUMeterCommonInit(this); CPUMeterData* data = this->meterData; Meter** meters = data->meters; this->mode = mode; - int start, count; + unsigned int start, count; AllCPUsMeter_getRange(this, &start, &count); if (!count) { this->h = 1; return; } - for (int i = 0; i < count; i++) { + for (unsigned int i = 0; i < count; i++) { Meter_setMode(meters[i], mode); } int h = meters[0]->h; @@ -350,17 +349,18 @@ static void OctoColCPUsMeter_updateMode(Meter* this, MeterModeId mode) { CPUMeterCommonUpdateMode(this, mode, 8); } -static void CPUMeterCommonDraw(Meter* this, int x, int y, int w, int ncol) { +static void CPUMeterCommonDraw(Meter* this, int x, int y, int w, unsigned int ncol) { CPUMeterData* data = this->meterData; Meter** meters = data->meters; - int start, count; + unsigned int start, count; AllCPUsMeter_getRange(this, &start, &count); int colwidth = w / ncol; int diff = w % ncol; - int nrows = (count + ncol - 1) / ncol; - for (int i = 0; i < count; i++) { - int d = (i / nrows) > diff ? diff : (i / nrows); // dynamic spacer - int xpos = x + ((i / nrows) * colwidth) + d; + unsigned int nrows = (count + ncol - 1) / ncol; + for (unsigned int i = 0; i < count; i++) { + int col = i / nrows; + int d = col > diff ? diff : col; // dynamic spacer + int xpos = x + (col * colwidth) + d; int ypos = y + ((i % nrows) * meters[0]->h); meters[i]->draw(meters[i], xpos, ypos, colwidth); } @@ -382,9 +382,9 @@ static void OctoColCPUsMeter_draw(Meter* this, int x, int y, int w) { static void SingleColCPUsMeter_draw(Meter* this, int x, int y, int w) { CPUMeterData* data = this->meterData; Meter** meters = data->meters; - int start, count; + unsigned int start, count; AllCPUsMeter_getRange(this, &start, &count); - for (int i = 0; i < count; i++) { + for (unsigned int i = 0; i < count; i++) { meters[i]->draw(meters[i], x, y, w); y += meters[i]->h; } From 73cf23cff8195300c79b34c6f93ca410fd2416c6 Mon Sep 17 00:00:00 2001 From: Daniel Lange Date: Wed, 24 Jun 2026 20:38:39 +0200 Subject: [PATCH 3/3] Address review feedback from @Explorer09 o Use xReallocArrayZero o Fix more unsigned int casting --- CPUMeter.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/CPUMeter.c b/CPUMeter.c index ed4fee4e2..90ec520a2 100644 --- a/CPUMeter.c +++ b/CPUMeter.c @@ -271,10 +271,7 @@ static void CPUMeterCommonInit(Meter* this) { Meter_delete((Object*)data->meters[i]); if (count > 0) { - data->meters = xReallocArray(data->meters, count, sizeof(Meter*)); - /* zero-fill newly added entries */ - if (count > prevCount) - memset(data->meters + prevCount, 0, (count - prevCount) * sizeof(Meter*)); + data->meters = xReallocArrayZero(data->meters, prevCount, count, sizeof(Meter*)); } else { free(data->meters); data->meters = NULL; @@ -354,13 +351,13 @@ static void CPUMeterCommonDraw(Meter* this, int x, int y, int w, unsigned int nc Meter** meters = data->meters; unsigned int start, count; AllCPUsMeter_getRange(this, &start, &count); - int colwidth = w / ncol; - int diff = w % ncol; + int colwidth = w / (int)ncol; + int diff = w % (int)ncol; unsigned int nrows = (count + ncol - 1) / ncol; for (unsigned int i = 0; i < count; i++) { - int col = i / nrows; - int d = col > diff ? diff : col; // dynamic spacer - int xpos = x + (col * colwidth) + d; + unsigned int col = i / nrows; + int d = (int)col > diff ? diff : (int)col; // dynamic spacer + int xpos = x + ((int)col * colwidth) + d; int ypos = y + ((i % nrows) * meters[0]->h); meters[i]->draw(meters[i], xpos, ypos, colwidth); }