[PATCH] cpupower: Avoid uninitialized reads in topology sorting

[Ali Ahmet Memis]

get_cpu_topology() allocates core_info with malloc(). If a topology
attribute disappears while CPUs are being hotplugged, an error path can
leave core_cpu_list uninitialized. __compare_core_cpu_list() then passes
the field to strcmp() while sorting the array.

Use calloc() for the array and ignore entries without complete topology
data when counting cores. Besides avoiding the invalid read, this keeps an
incomplete entry from being counted as a physical core.

Fixes: f89cb9cba7a2 ("cpupower: Implement CPU physical core querying")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Ali Ahmet Memis <ali@xxxxxxxxxxxxxx>
---
Tested with a two-CPU topology mock that makes the second CPU's sysfs
reads fail. Valgrind reports uninitialized reads before this patch and
no errors after it.

Build-tested with:
  make -C tools/power/cpupower NLS=false CPUFREQ_BENCH=false

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

diff --git a/tools/power/cpupower/lib/cpupower.c b/tools/power/cpupower/lib/cpupower.c
index d7f7ec6f1..a8ee304bd 100644
--- a/tools/power/cpupower/lib/cpupower.c
+++ b/tools/power/cpupower/lib/cpupower.c
@@ -171,7 +171,7 @@ int get_cpu_topology(struct cpupower_topology *cpu_top)
 	char path[SYSFS_PATH_MAX];
 	char *last_cpu_list;
 
-	cpu_top->core_info = malloc(sizeof(struct cpuid_core_info) * cpus);
+	cpu_top->core_info = calloc(cpus, sizeof(struct cpuid_core_info));
 	if (cpu_top->core_info == NULL)
 		return -ENOMEM;
 	cpu_top->pkgs = cpu_top->cores = 0;
@@ -214,11 +214,17 @@ int get_cpu_topology(struct cpupower_topology *cpu_top)
 	qsort(cpu_top->core_info, cpus, sizeof(struct cpuid_core_info),
 	      __compare_core_cpu_list);
 
-	last_cpu_list = cpu_top->core_info[0].core_cpu_list;
-	cpu_top->cores = 1;
-	for (cpu = 1; cpu < cpus; cpu++) {
-		if (strcmp(cpu_top->core_info[cpu].core_cpu_list, last_cpu_list) != 0 &&
-		    cpu_top->core_info[cpu].pkg != -1) {
+	last_cpu_list = NULL;
+	cpu_top->cores = 0;
+	for (cpu = 0; cpu < cpus; cpu++) {
+		if (cpu_top->core_info[cpu].pkg == -1 ||
+		    cpu_top->core_info[cpu].core == -1 ||
+		    cpu_top->core_info[cpu].core_cpu_list[0] == '\0')
+			continue;
+
+		if (!last_cpu_list ||
+		    strcmp(cpu_top->core_info[cpu].core_cpu_list,
+			   last_cpu_list) != 0) {
 			last_cpu_list = cpu_top->core_info[cpu].core_cpu_list;
 			cpu_top->cores++;
 		}
-- 
2.54.0