Hi Peter,
On Fri, May 08, 2026 at 10:15:10AM +0100, Peter Maydell wrote:
> On Wed, 29 Apr 2026 at 19:35, Harsh Prateek Bora <[email protected]>
> wrote:
> >
> > From: Aditya Gupta <[email protected]>
> >
> > Set the "Thread Register State Entry Size" that is required by firmware
> > (OPAL), to know size of memory to allocate to capture CPU state, in the
> > event of a crash
> >
> > Reviewed-by: Hari Bathini <[email protected]>
> > Reviewed-by: Sourabh Jain <[email protected]>
> > Signed-off-by: Aditya Gupta <[email protected]>
> > Tested-by: Shivang Upadhyay <[email protected]>
> > Link:
> > https://lore.kernel.org/qemu-devel/[email protected]
> > Signed-off-by: Harsh Prateek Bora <[email protected]>
>
> Hi; Coverity points out an issue with this change (CID 1658041):
Thanks for reporting, I'll look into this.
>
> > static void pnv_reset(MachineState *machine, ResetType type)
> > {
> > + PnvMachineState *pnv = PNV_MACHINE(machine);
> > void *fdt;
> >
> > qemu_devices_reset(type);
> >
> > + if (!pnv->mpipl_state.is_next_boot_mpipl) {
> > + /*
> > + * Set the "Thread Register State Entry Size", so that firmware can
> > + * allocate enough memory to capture CPU state in the event of a
> > + * crash
> > + */
> > +
> > + MpiplProcDumpArea proc_area;
>
> Here we don't initialize the struct...
>
> > +
> > + proc_area.version = PROC_DUMP_AREA_VERSION_P9;
> > + proc_area.thread_size =
> > cpu_to_be32(sizeof(MpiplPreservedCPUState));
> > +
> > + /* These are to be allocated & assigned by the firmware */
> > + proc_area.alloc_addr = 0;
> > + proc_area.alloc_size = 0;
> > +
> > + /* These get assigned after crash, when QEMU preserves the
> > registers */
> > + proc_area.dest_addr = 0;
> > + proc_area.act_size = 0;
>
> ...and here we don't fill in all the fields; we don't set
> the reserved, reserved2 or reserved3 fields to anything.
> This means that we will write data to the guest which is
> potentially random host data from the stack.
>
> I think I'd suggest fixing this by initializing the struct in
> one go, like this:
>
> MpiplProcDumpArea proc_area = {
> .version = PROC_DUMP_AREA_VERSION_P9,
> .thread_size = cpu_to_be32(sizeof(MpiplPreservedCPUState)),
> [set alloc_addr = 0 etc here if you like, or rely on
> the "fields not listed are zero-inited"]
> };
>
> (In fact because we use -ftrivial-auto-var-init=zero the compiler
> will zero init this for us anyway, but we can make Coverity
> happy anyway.)
Thanks for suggesting the fix too.
~Shivang.
