On 30/10/2018 07:28, P J P wrote: > From: Prasad J Pandit <p...@fedoraproject.org> > > While writing a message in 'lsi_do_msgin', message length value > in 'msg_len' could be invalid. Add check to avoid OOB access issue. > > Signed-off-by: Prasad J Pandit <p...@fedoraproject.org>
Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> with one change below: > --- > hw/scsi/lsi53c895a.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > Update v2: modify assert() > -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg06273.html > > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c > index d1e6534311..dc39f4c3ee 100644 > --- a/hw/scsi/lsi53c895a.c > +++ b/hw/scsi/lsi53c895a.c > @@ -861,10 +861,11 @@ static void lsi_do_status(LSIState *s) > > static void lsi_do_msgin(LSIState *s) > { > - int len; > + uint8_t len; > trace_lsi_do_msgin(s->dbc, s->msg_len); > s->sfbr = s->msg[0]; > len = s->msg_len; > + assert(len > 0 && len <= LSI_MAX_MSGIN_LEN); > if (len > s->dbc) > len = s->dbc; > pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len); > @@ -1705,8 +1706,10 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset) > break; > case 0x58: /* SBDL */ > /* Some drivers peek at the data bus during the MSG IN phase. */ > - if ((s->sstat1 & PHASE_MASK) == PHASE_MI) > + if ((s->sstat1 & PHASE_MASK) == PHASE_MI) { > + assert(s->msg_len >= 0); should be > 0 as well. Paolo > return s->msg[0]; > + } > ret = 0; > break; > case 0x59: /* SBDL high */ > @@ -2103,11 +2106,23 @@ static int lsi_pre_save(void *opaque) > return 0; > } > > +static int lsi_post_load(void *opaque, int version_id) > +{ > + LSIState *s = opaque; > + > + if (s->msg_len < 0 || s->msg_len > LSI_MAX_MSGIN_LEN) { > + return -EINVAL; > + } > + > + return 0; > +} > + > static const VMStateDescription vmstate_lsi_scsi = { > .name = "lsiscsi", > .version_id = 0, > .minimum_version_id = 0, > .pre_save = lsi_pre_save, > + .post_load = lsi_post_load, > .fields = (VMStateField[]) { > VMSTATE_PCI_DEVICE(parent_obj, LSIState), > >
