@samitouri / QOSamiQemu / commits / efbf01e03f

hw/dma/omap_dma: Be more careful about overflow in transfer setup

In omap_dma_transfer_setup(), the maximum number of elements we can transfer is 0xffff * 0xffff == 0xfffe0001 (because the max frame count and max elements per frame are both 65535). However, we store total element counts in 'int' variables, and use INT_MAX as a "bigger than any valid value" sentinel, and when performing arithmetic with the total count of transferred elements we are not careful about avoiding overflows. Fix these: - use uint32_t rather than int for the local variables tracking various element and frame counts - use UINT_MAX as our sentinel - calculate new packet, element and frame counter values using arithmetic on a local uint32_t, rather than doing it in-place on local variables that are only 'int' because the actual counter registers are 16 bits - use 64-bit arithmetic when calculating how much to advance the source and dest pointers and the total dma->bytes transferred Note that since soc_dma_ch_s::bytes is only 'int' this can still overflow; we'll fix that in a subsequent patch. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org> Message-id: 20260710105907.2570621-5-peter.maydell@linaro.org

Peter Maydell committed Jul 20, 2026 at 19:05 UTC efbf01e03fb0fa0f473898d33349cdd6f5946a92
1 file changed +20 -10
hw/dma/omap_dma.c
+20 -10
@@ -391,7 +391,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
391 struct omap_dma_reg_set_s *a;
392 struct omap_dma_channel_s *ch = dma->opaque;
393 struct omap_dma_s *s = dma->dma->opaque;
394 - int frames, min_elems, elements[__omap_dma_intr_last];
394 + uint32_t frames, min_elems, elements[__omap_dma_intr_last];
395
396 a = &ch->active_set;
397
@@ -403,7 +403,14 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
403 __func__, dma->num);
404 }
405
406 - min_elems = INT_MAX;
406 + /*
407 + * The maximum frame count and maximum element count are both 0xffff,
408 + * so our worst case possible number of elements to transfer is
409 + * 0xffff * 0xffff == 0xfffe0001. We can therefore keep element
410 + * counts in a uint32_t and use UINT_MAX as a sentinel value for
411 + * "not set" / "condition does not occur".
412 + */
413 + min_elems = UINT_MAX;
414
415 /* Check all the conditions that terminate the transfer starting
416 * with those that can occur the soonest. */
@@ -413,7 +420,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
420 if (elements[id] < min_elems) \
421 min_elems = elements[id]; \
422 } else \
416 - elements[id] = INT_MAX;
423 + elements[id] = UINT_MAX;
424
425 /* Elements */
426 INTR_CHECK(
@@ -465,7 +472,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
472 (a->frames - a->frame - 1) * a->elements +
473 (a->elements - a->element))
474
468 - dma->bytes = min_elems * ch->data_type;
475 + dma->bytes = (uint64_t)min_elems * ch->data_type;
476
477 /* Set appropriate interrupts and/or deactivate channels */
478
@@ -528,8 +535,9 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
535
536 /* Update packet number */
537 if (ch->fs && ch->bs) {
531 - a->pck_element += min_elems;
532 - a->pck_element %= a->pck_elements;
538 + /* Can't overflow: worst case min_elems 0xFFFE0001 + element 0xFFFF */
539 + uint32_t new_pck_element = a->pck_element + min_elems;
540 + a->pck_element = new_pck_element % a->pck_elements;
541 }
542
543 /*
@@ -537,13 +545,15 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma)
545 * can skip part of this.
546 */
547 if (dma->update) {
548 + /* Can't overflow: worst case min_elems 0xFFFE0001 + element 0xFFFF */
549 + uint32_t new_element = a->element + min_elems;
550 a->element += min_elems;
551
542 - frames = a->element / a->elements;
543 - a->element = a->element % a->elements;
552 + frames = new_element / a->elements;
553 + a->element = new_element % a->elements;
554 a->frame += frames;
545 - a->src += min_elems * a->elem_delta[0] + frames * a->frame_delta[0];
546 - a->dest += min_elems * a->elem_delta[1] + frames * a->frame_delta[1];
555 + a->src += (uint64_t)min_elems * a->elem_delta[0] + frames * a->frame_delta[0];
556 + a->dest += (uint64_t)min_elems * a->elem_delta[1] + frames * a->frame_delta[1];
557
558 /* If the channel is async, update cpc */
559 if (!ch->sync && frames) {