On Tue, 10 Mar 2020 at 15:09, Christian Borntraeger <borntrae...@de.ibm.com> wrote: > > From: Halil Pasic <pa...@linux.ibm.com> > > We expose loadparm as a r/w machine property, but if loadparm is set by > the guest via DIAG 308, we don't update the property. Having a > disconnect between the guest view and the QEMU property is not nice in > itself, but things get even worse for SCSI, where under certain > circumstances (see 789b5a401b "s390: Ensure IPL from SCSI works as > expected" for details) we call s390_gen_initial_iplb() on resets > effectively overwriting the guest/user supplied loadparm with the stale > value.
Hi; Coverity points out (CID 1421966) that you have a buffer overrun here: > +static void update_machine_ipl_properties(IplParameterBlock *iplb) > +{ > + Object *machine = qdev_get_machine(); > + Error *err = NULL; > + > + /* Sync loadparm */ > + if (iplb->flags & DIAG308_FLAGS_LP_VALID) { > + uint8_t *ebcdic_loadparm = iplb->loadparm; > + char ascii_loadparm[8]; This array is 8 bytes... > + int i; > + > + for (i = 0; i < 8 && ebcdic_loadparm[i]; i++) { > + ascii_loadparm[i] = ebcdic2ascii[(uint8_t) ebcdic_loadparm[i]]; > + } > + ascii_loadparm[i] = 0; ...but you can write 9 bytes into it (8 from the guest-controlled iplb_loadparm buffer plus one for the trailing NUL). > + object_property_set_str(machine, ascii_loadparm, "loadparm", &err); > + } else { > + object_property_set_str(machine, "", "loadparm", &err); > + } > + if (err) { > + warn_report_err(err); > + } > +} thanks -- PMM