| Message ID | 20250319020847.1511759-5-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/loongarch: Solve some issues reported from coccinelle | expand |
Bibo Mao <maobibo@loongson.cn> writes: > In function virt_cpu_irq_init(), there is notification with ipi and extioi > interrupt controller for cpu creation. Local variable with error type is > used, however there is no check with its return value. Good catch. When the first call fails, we pass non-null @err to the second call, which is wrong. If that one also fails, it'll likely trip error_setv()'s assertion. > Here set dest error object with error_abort, rather than local variable, so > application will abort to run if there is error. Why is failure impossible there? If failure is impossible, the code before the patch is harmlessly wrong. If failure is possible, the code before the patch has a crash bug, and the patch makes it crash harder, i.e. when either call fails instead of when both fail. > Fixes: 50ebc3fc47fe (hw/intc/loongarch_ipi: Notify ipi object when cpu is plugged) > Fixes: 087a23a87c57 (hw/intc/loongarch_extioi: Use cpu plug notification) > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > --- > hw/loongarch/virt.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c > index a9fab39dd8..f10a4704ab 100644 > --- a/hw/loongarch/virt.c > +++ b/hw/loongarch/virt.c > @@ -327,7 +327,6 @@ static void virt_cpu_irq_init(LoongArchVirtMachineState *lvms) > MachineClass *mc = MACHINE_GET_CLASS(ms); > const CPUArchIdList *possible_cpus; > CPUState *cs; > - Error *err = NULL; > > /* cpu nodes */ > possible_cpus = mc->possible_cpu_arch_ids(ms); > @@ -337,8 +336,10 @@ static void virt_cpu_irq_init(LoongArchVirtMachineState *lvms) > continue; > } > > - hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), DEVICE(cs), &err); > - hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), DEVICE(cs), &err); > + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), DEVICE(cs), > + &error_abort); > + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), DEVICE(cs), > + &error_abort); > } > }
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index a9fab39dd8..f10a4704ab 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -327,7 +327,6 @@ static void virt_cpu_irq_init(LoongArchVirtMachineState *lvms) MachineClass *mc = MACHINE_GET_CLASS(ms); const CPUArchIdList *possible_cpus; CPUState *cs; - Error *err = NULL; /* cpu nodes */ possible_cpus = mc->possible_cpu_arch_ids(ms); @@ -337,8 +336,10 @@ static void virt_cpu_irq_init(LoongArchVirtMachineState *lvms) continue; } - hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), DEVICE(cs), &err); - hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), DEVICE(cs), &err); + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), DEVICE(cs), + &error_abort); + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), DEVICE(cs), + &error_abort); } }
In function virt_cpu_irq_init(), there is notification with ipi and extioi interrupt controller for cpu creation. Local variable with error type is used, however there is no check with its return value. Here set dest error object with error_abort, rather than local variable, so application will abort to run if there is error. Fixes: 50ebc3fc47fe (hw/intc/loongarch_ipi: Notify ipi object when cpu is plugged) Fixes: 087a23a87c57 (hw/intc/loongarch_extioi: Use cpu plug notification) Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- hw/loongarch/virt.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)