Fix cpumeter segfault#2025
Conversation
…rs 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: htop-dev#2024 Assisted-by: Microsoft Copilot/Claude Sonnet 4.6
Assisted-by: Microsoft Copilot/Claude Sonnet 4.6
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes identified. Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
BenBE
added
bug 🐛
| 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*)); |
There was a problem hiding this comment.
Consider using xReallocArrayZero.
| assert(count >= 0); | ||
| unsigned int prevCount = data->cpus; | ||
| if ((unsigned int)count != prevCount) { | ||
| prevCount = data->cpus; |
There was a problem hiding this comment.
The declaration of prevCount should be moved back here. There's no use of prevCount before this line.
| 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; |
There was a problem hiding this comment.
There's an implicit cast from unsigned int to int here.
o Use xReallocArrayZero o Fix more unsigned int casting
3 participants
Fixes: #2024
and some int -> unsigned int mopping up old code (2nd commit)