[PATCH v2 0/2] cpupower: fix topology array handling

Ali Ahmet Memis <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.power-management.general
Message-ID <[email protected]>
v1 did two things in one patch. Shuah asked for them to be split, so here
they are as two.

Patch 1 is the uninitialized read: get_cpu_topology() allocates core_info
with malloc(), several paths never write core_cpu_list, and the sort
comparator then hands that buffer to strcmp(). calloc() fixes it.

Patch 2 is separate and only about the physical core count. The counting
loop seeds cores at 1 from entry 0 without checking whether that entry has
usable topology data, so an incomplete entry can be counted as a core.
Patch 2 depends on patch 1, since it uses an empty core_cpu_list to
recognise an entry that was never filled in.

Tested against a fake sysfs tree with the configured CPU count pinned to 2,
where cpu0 has complete topology and cpu1's topology attributes are absent.
Same harness, one commit apart:

  unpatched     cores=2   valgrind: errors
  patch 1       cores=2   valgrind: clean
  patch 1+2     cores=1   valgrind: clean

Unpatched, valgrind traces it to the allocation:

  Conditional jump or move depends on uninitialised value(s)
     at strcmp (vg_replace_strmem.c:941)
     by __compare_core_cpu_list (cpupower.c:159)
     by qsort_r (qsort.c:409)
     by get_cpu_topology (cpupower.c:214)
   Uninitialised value was created by a heap allocation
     at malloc (vg_replace_malloc.c:446)
     by get_cpu_topology (cpupower.c:174)

I have no machine where a topology attribute actually disappears during
enumeration, so the fake tree is as close as I could get. If you would
rather see this exercised some other way, say so and I will do that.

Link to the v1 review:
https://lore.kernel.org/all/[email protected]/

Ali Ahmet Memis (2):
  cpupower: zero the topology array to avoid uninitialized reads
  cpupower: do not count incomplete topology entries as physical cores

 tools/power/cpupower/lib/cpupower.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.