From: Alexander Bluhm <bluhm@openbsd.org>
Subject: bus dma bounce buffer contiguous virtual address
To: tech@openbsd.org
Date: Wed, 21 Aug 2024 19:21:31 +0200

Hi,

Together with hshoexer@ we found another bug in the bus dma bounce
buffer implementation.  If the physical pages are contiguous,
_bus_dmamap_load_buffer() tries to merge the segments.  In case of
mbuf chains, it can happen that the physical bounce buffers are
contiguous, but the virtual addresses the mbuf m_data are not.  Then
during transmit _bus_dmamap_sync() tries to copy segments where it
cannot access the virtual source address which is mapped in a
different mbuf.  So if bounce buffers are used, physical and virtual
buffer must be contigous, to merge a segment.

While there split the check and decrement of variable i in the for
loop to make the code readable.

ok?

bluhm

Index: arch/amd64/amd64/bus_dma.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/bus_dma.c,v
diff -u -p -r1.56 bus_dma.c
--- arch/amd64/amd64/bus_dma.c	20 Aug 2024 15:30:29 -0000	1.56
+++ arch/amd64/amd64/bus_dma.c	21 Aug 2024 16:37:31 -0000
@@ -499,7 +499,7 @@ _bus_dmamap_sync(bus_dma_tag_t t, bus_dm
 	if (!use_bounce_buffer)
 		return;
 
-	for (i = map->_dm_segcnt, sg = map->dm_segs; size && i--; sg++) {
+	for (i = map->_dm_segcnt, sg = map->dm_segs; size && i; i--, sg++) {
 		if (off >= sg->ds_len) {
 			off -= sg->ds_len;
 			continue;
@@ -767,9 +767,11 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
 			     map->_dm_maxsegsz &&
 			    (map->_dm_boundary == 0 ||
 			     (map->dm_segs[seg].ds_addr & bmask) ==
-			     (curaddr & bmask)))
+			     (curaddr & bmask)) &&
+			    (!use_bounce_buffer || (map->dm_segs[seg]._ds_va +
+			     map->dm_segs[seg].ds_len) == vaddr)) {
 				map->dm_segs[seg].ds_len += sgsize;
-			else {
+			} else {
 				if (++seg >= map->_dm_segcnt)
 					break;
 				map->dm_segs[seg].ds_addr = curaddr;