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

On 8/3/26 11:52, Ali Ahmet Memis wrote:
> 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.

I agree that core_cpu_list isn't initialized and that needs fixing.
It can be done with your first patch that replaces malloc() with
calloc().

This code path can be improved to initialize core_cpu_list.

Did you think about a scenario when the following check will
be tru - i.e core == -1 is trur?

if (cpu_top->core_info[cpu].core == -1) {
                         strncpy(cpu_top->core_info[cpu].core_cpu_list, "-1", CPULIST_BUFFER);
                         continue;
                 }

> 
> 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.

Can you elaborate on a real scenario where this could happen after
replacing malloc() with calloc() and making sure core_cpu_list is
initialized to "-1" like in the above conditional?

thanks,
-- Shuah