From: Ian Rogers irogers@google.com
[ Upstream commit a0a4173631bfcfd3520192c0a61cf911d6a52c3a ]
Passing an empty map to perf_cpu_map__max triggered a SEGV. Explicitly test for the empty map.
Reported-by: Ingo Molnar mingo@kernel.org Closes: https://lore.kernel.org/linux-perf-users/aSwt7yzFjVJCEmVp@gmail.com/ Tested-by: Ingo Molnar mingo@kernel.org Signed-off-by: Ian Rogers irogers@google.com Tested-by: Thomas Richter tmricht@linux.ibm.com Signed-off-by: Namhyung Kim namhyung@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Commit Analysis: libperf cpumap: Fix perf_cpu_map__max for an empty/NULL map
### 1. COMMIT MESSAGE ANALYSIS
- **Subject**: Clearly indicates a bug fix for `perf_cpu_map__max()` - **Bug type**: SEGV (segmentation fault / crash) - a severe issue - **Tags present**: - `Reported-by: Ingo Molnar` - reported by a prominent kernel developer - `Closes:` link to mailing list bug report - `Tested-by:` from two people (Ingo Molnar and Thomas Richter) - `Signed-off-by:` from maintainers - **Missing tags**: No explicit `Cc: stable@vger.kernel.org` or `Fixes:` tag
### 2. CODE CHANGE ANALYSIS
**Before the fix:** ```c return __perf_cpu_map__nr(map) > 0 ? __perf_cpu_map__cpu(map, __perf_cpu_map__nr(map) - 1) : result; ```
**After the fix:** ```c if (!map) return result;
// The CPUs are always sorted and nr is always > 0 as 0 length map is // encoded as NULL. return __perf_cpu_map__cpu(map, __perf_cpu_map__nr(map) - 1); ```
**The Bug Mechanism:** The old code attempted to check `__perf_cpu_map__nr(map) > 0` before accessing the map, but this check **itself** dereferences the `map` pointer. When `map` is NULL, calling `__perf_cpu_map__nr(map)` results in a NULL pointer dereference, causing a SEGV.
**The Fix:** - Adds an explicit NULL check at the function entry point - Returns the default result (`cpu = -1`) for NULL maps - Simplifies the logic based on the invariant that a 0-length map is encoded as NULL (so if map is non-NULL, it has at least one entry)
### 3. CLASSIFICATION
- **Type**: Bug fix (NULL pointer dereference causing crash) - **Not a feature**: Doesn't add new functionality - **Not a quirk/device ID**: Pure crash fix in existing code
### 4. SCOPE AND RISK ASSESSMENT
| Metric | Assessment | |--------|------------| | Lines changed | ~10 (very small) | | Files touched | 1 file | | Complexity | Very low - simple NULL check | | Subsystem | tools/lib/perf (perf userspace tooling) | | Risk level | **Very low** |
The fix is defensive - adding a NULL check cannot break anything that was previously working.
### 5. USER IMPACT
- **Affected users**: Anyone using perf tooling - **Trigger condition**: Passing an empty/NULL cpu map to `perf_cpu_map__max()` - **Severity**: Crash (SEGV) - high severity - **Reproducibility**: Reliably reproducible per the bug report
### 6. STABILITY INDICATORS
- ✅ Tested by Ingo Molnar (original reporter) - ✅ Tested by Thomas Richter (IBM) - ✅ Signed off by maintainer Namhyung Kim - ✅ Simple, obviously correct fix - ✅ Well-documented reasoning in updated comment
### 7. DEPENDENCY CHECK
- Self-contained fix - no dependencies on other commits - Uses existing internal functions (`__perf_cpu_map__nr`, `__perf_cpu_map__cpu`) that would exist in stable trees - Should apply cleanly to stable trees
### STABLE CRITERIA EVALUATION
| Criterion | Met? | |-----------|------| | Obviously correct | ✅ Yes - simple NULL check | | Fixes real bug | ✅ Yes - SEGV crash | | Important issue | ✅ Yes - crash in widely-used tool | | Small and contained | ✅ Yes - few lines, one function | | No new features | ✅ Yes - purely defensive | | No new APIs | ✅ Yes |
### CONCLUSION
This commit is an excellent candidate for stable backport:
1. **Fixes a real crash**: SEGV when passing NULL/empty map is a serious bug that users can hit 2. **Obviously correct**: Simple NULL check at function entry is textbook defensive programming 3. **Minimal risk**: Cannot cause regressions - only affects the previously-broken case 4. **Well tested**: Multiple test acknowledgments from respected developers 5. **Small and surgical**: Touches only one function in one file 6. **No dependencies**: Self-contained fix
The missing `Cc: stable` tag appears to be an oversight rather than an indication the fix isn't appropriate for stable. The perf tooling is widely used, and crash fixes in it are important for stable users.
**YES**
tools/lib/perf/cpumap.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c index b20a5280f2b33..2bbbe1c782b8a 100644 --- a/tools/lib/perf/cpumap.c +++ b/tools/lib/perf/cpumap.c @@ -368,10 +368,12 @@ struct perf_cpu perf_cpu_map__max(const struct perf_cpu_map *map) .cpu = -1 };
- // cpu_map__trim_new() qsort()s it, cpu_map__default_new() sorts it as well. - return __perf_cpu_map__nr(map) > 0 - ? __perf_cpu_map__cpu(map, __perf_cpu_map__nr(map) - 1) - : result; + if (!map) + return result; + + // The CPUs are always sorted and nr is always > 0 as 0 length map is + // encoded as NULL. + return __perf_cpu_map__cpu(map, __perf_cpu_map__nr(map) - 1); }
/** Is 'b' a subset of 'a'. */