Jim MacArthur <[email protected]> writes: > If both frame and element count are 65535, which appears valid from my > reading of the OMAP5912 documentation, then some of the calculations > will overflow the 32-bit signed integer range and produce a negative > min_elems value. > > Raised by #3204 (https://gitlab.com/qemu-project/qemu/-/issues/3204). >
nit: Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3204 > Signed-off-by: Jim MacArthur <[email protected]> > --- > hw/dma/omap_dma.c | 32 +++++++++++++++++++++----------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c > index 101f91f4a3..93e6503ff9 100644 > --- a/hw/dma/omap_dma.c > +++ b/hw/dma/omap_dma.c > @@ -504,9 +504,19 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s > *dma) > struct omap_dma_channel_s *ch = dma->opaque; > struct omap_dma_s *s = dma->dma->opaque; > int frames, min_elems, elements[__omap_dma_intr_last]; > + uint64_t frames64, frame64, elements64, element64; > > a = &ch->active_set; > > + /* > + * We do maths with the frame and element fields which exceeds > + * a signed 32-bit integer, so convert all these to 64 bit for future > use. > + */ > + frames64 = a->frames; > + frame64 = a->frame; > + elements64 = a->elements; > + element64 = a->element; > + > src_p = &s->mpu->port[ch->port[0]]; > dest_p = &s->mpu->port[ch->port[1]]; > if ((!ch->constant_fill && !src_p->addr_valid(s->mpu, a->src)) || > @@ -527,7 +537,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s > *dma) > /* Check all the conditions that terminate the transfer starting > * with those that can occur the soonest. */ > #define INTR_CHECK(cond, id, nelements) \ > - if (cond) { \ > + if (cond && nelements <= INT_MAX) { \ > elements[id] = nelements; \ > if (elements[id] < min_elems) \ > min_elems = elements[id]; \ > @@ -547,24 +557,24 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s > *dma) > * See also the TODO in omap_dma_channel_load. */ > INTR_CHECK( > (ch->interrupts & LAST_FRAME_INTR) && > - ((a->frame < a->frames - 1) || !a->element), > + ((frame64 < frames64 - 1) || !element64), > omap_dma_intr_last_frame, > - (a->frames - a->frame - 2) * a->elements + > - (a->elements - a->element + 1)) > + (frames64 - frame64 - 2) * elements64 + > + (elements64 - element64 + 1)) > INTR_CHECK( > ch->interrupts & HALF_FRAME_INTR, > omap_dma_intr_half_frame, > - (a->elements >> 1) + > - (a->element >= (a->elements >> 1) ? a->elements : 0) - > - a->element) > + (elements64 >> 1) + > + (element64 >= (elements64 >> 1) ? elements64 : 0) - > + element64) > INTR_CHECK( > ch->sync && ch->fs && (ch->interrupts & END_FRAME_INTR), > omap_dma_intr_frame, > - a->elements - a->element) > + elements64 - element64) > INTR_CHECK( > ch->sync && ch->fs && !ch->bs, > omap_dma_intr_frame_sync, > - a->elements - a->element) > + elements64 - element64) > > /* Packets */ > INTR_CHECK( > @@ -581,8 +591,8 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s > *dma) > INTR_CHECK( > 1, > omap_dma_intr_block, > - (a->frames - a->frame - 1) * a->elements + > - (a->elements - a->element)) > + (frames64 - frame64 - 1) * elements64 + > + (elements64 - element64)) > > dma->bytes = min_elems * ch->data_type; Can we also add a qtest for the device that checks for this (and can be expanded for other unit tests later)? -- Alex Bennée Virtualisation Tech Lead @ Linaro
