[Skiboot] [PATCH] core/cpu: Fix memory allocation for job array

Oliver oohall at gmail.com
Mon Sep 3 15:53:27 AEST 2018


On Mon, Sep 3, 2018 at 3:37 PM, Oliver <oohall at gmail.com> wrote:
> On Sun, Sep 2, 2018 at 7:59 PM, Vaidyanathan Srinivasan
> <svaidy at linux.vnet.ibm.com> wrote:
>> fixes: 7a3f307e core/cpu: parallelise global CPU register setting jobs
>>
>> This bug would result in boot-hang on some configurations due to
>> cpu_wait_job() endlessly waiting for the last bogus jobs[cpu->pir] pointer.
>
> Under what circumstances does it fail? The minimum allocation block is
> sizeof(long) so even with the undersized allocation it I would expect
> it to still work. I might be missing something.

Pointers are hard. Ignore me.

>
>> Reported-by: Stephanie Swanson <swanman at us.ibm.com>
>> Reported-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>> Signed-off-by: Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com>
>> ---
>>  core/cpu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/core/cpu.c b/core/cpu.c
>> index 88477f82..cc5fba32 100644
>> --- a/core/cpu.c
>> +++ b/core/cpu.c
>> @@ -1373,7 +1373,7 @@ static int64_t cpu_change_all_hid0(struct hid0_change_req *req)
>>         struct cpu_thread *cpu;
>>         struct cpu_job **jobs;
>>
>> -       jobs = zalloc(sizeof(struct cpu_job *) * cpu_max_pir + 1);
>> +       jobs = zalloc(sizeof(struct cpu_job *) * (cpu_max_pir + 1));
>
> Looks like this code was copied and pasted form cpu_cleanup_all()
> since the same bug exists there. Can you add a fix there too?
>
> We might want to just refactor this so that we have a common
> cpu_run_on_all() or something. I'm not too bothered though.
>
>>         assert(jobs);
>>
>>         for_each_available_cpu(cpu) {
>> --
>> 2.17.1
>>
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot


More information about the Skiboot mailing list