| Message ID | 20170724182751.18261-21-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > In a ARM multicore system, write_secondary_boot() only initializes fixups for > FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, while smpboot[] also uses FIXUP_DSB. > This results in write_bootloader() using uninitialized fixupcontext[FIXUP_DSB] > instruction in the bootloader code... Hmm? The code does: if (arm_feature(&cpu->env, ARM_FEATURE_V7)) { fixupcontext[FIXUP_DSB] = DSB_INSN; } else { fixupcontext[FIXUP_DSB] = CP15_DSB_INSN; } so fixupcontext[FIXUP_DSB] is guaranteed initialized, as are FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, which are the only fixups that the smpboot[] code uses. thanks -- PMM
On 07/24/2017 06:06 PM, Peter Maydell wrote: > On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> In a ARM multicore system, write_secondary_boot() only initializes fixups for >> FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, while smpboot[] also uses FIXUP_DSB. >> This results in write_bootloader() using uninitialized fixupcontext[FIXUP_DSB] >> instruction in the bootloader code... > > Hmm? The code does: > > if (arm_feature(&cpu->env, ARM_FEATURE_V7)) { > fixupcontext[FIXUP_DSB] = DSB_INSN; > } else { > fixupcontext[FIXUP_DSB] = CP15_DSB_INSN; > } > > so fixupcontext[FIXUP_DSB] is guaranteed initialized, > as are FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, which are > the only fixups that the smpboot[] code uses. Indeed :) Sorry for the noise, I'll add few hints to the analyzer.
diff --git a/hw/arm/boot.c b/hw/arm/boot.c index c2720c8046..fb21f113c2 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -170,7 +170,7 @@ static void write_bootloader(const char *name, hwaddr addr, static void default_write_secondary(ARMCPU *cpu, const struct arm_boot_info *info) { - uint32_t fixupcontext[FIXUP_MAX]; + uint32_t fixupcontext[FIXUP_MAX] = {}; fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr; fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
In a ARM multicore system, write_secondary_boot() only initializes fixups for FIXUP_GIC_CPU_IF and FIXUP_BOOTREG, while smpboot[] also uses FIXUP_DSB. This results in write_bootloader() using uninitialized fixupcontext[FIXUP_DSB] instruction in the bootloader code... Zero-initialize fixupcontext[] to avoid this issue. hw/arm/boot.c:157:18: warning: Assigned value is garbage or undefined insn = fixupcontext[fixup]; ^ ~~~~~~~~~~~~~~~~~~~ Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/arm/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)