Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
bus dma bounce buffer pages used
To:
tech@openbsd.org
Date:
Mon, 19 Aug 2024 18:35:26 +0200

Download raw body.

Thread
Hi,

There is an off-by-one bug when comparing the used pages for bounce
buffers with the available pages.  So _bus_dmamap_load_buffer()
returns ENOMEM although there is one buffer left.

Also the _dm_nused field is updated and never reset in case of an
error.  Use a local variable to count the used pages and update
global map->_dm_nused only if there was no error.

This fixes a hanging network if bounce buffers are enforced for
vio(4).

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.53 bus_dma.c
--- arch/amd64/amd64/bus_dma.c	18 Aug 2024 21:04:29 -0000	1.53
+++ arch/amd64/amd64/bus_dma.c	19 Aug 2024 16:10:56 -0000
@@ -102,7 +102,7 @@
 #endif
 
 int _bus_dmamap_load_buffer(bus_dma_tag_t, bus_dmamap_t, void *, bus_size_t,
-    struct proc *, int, paddr_t *, int *, int);
+    struct proc *, int, paddr_t *, int *, int *, int);
 
 /*
  * Common function for DMA map creation.  May be called by bus-specific
@@ -251,7 +251,7 @@ _bus_dmamap_load(bus_dma_tag_t t, bus_dm
     bus_size_t buflen, struct proc *p, int flags)
 {
 	bus_addr_t lastaddr = 0;
-	int seg, error;
+	int seg, used, error;
 
 	/*
 	 * Make sure that on error condition we return "no valid mappings".
@@ -263,11 +263,13 @@ _bus_dmamap_load(bus_dma_tag_t t, bus_dm
 		return (EINVAL);
 
 	seg = 0;
+	used = 0;
 	error = _bus_dmamap_load_buffer(t, map, buf, buflen, p, flags,
-	    &lastaddr, &seg, 1);
+	    &lastaddr, &seg, &used, 1);
 	if (error == 0) {
 		map->dm_mapsize = buflen;
 		map->dm_nsegs = seg + 1;
+		map->_dm_nused = used;
 	}
 	return (error);
 }
@@ -280,7 +282,7 @@ _bus_dmamap_load_mbuf(bus_dma_tag_t t, b
     int flags)
 {
 	paddr_t lastaddr = 0;
-	int seg, error, first;
+	int seg, used, error, first;
 	struct mbuf *m;
 
 	/*
@@ -299,17 +301,19 @@ _bus_dmamap_load_mbuf(bus_dma_tag_t t, b
 
 	first = 1;
 	seg = 0;
+	used = 0;
 	error = 0;
 	for (m = m0; m != NULL && error == 0; m = m->m_next) {
 		if (m->m_len == 0)
 			continue;
 		error = _bus_dmamap_load_buffer(t, map, m->m_data, m->m_len,
-		    NULL, flags, &lastaddr, &seg, first);
+		    NULL, flags, &lastaddr, &seg, &used, first);
 		first = 0;
 	}
 	if (error == 0) {
 		map->dm_mapsize = m0->m_pkthdr.len;
 		map->dm_nsegs = seg + 1;
+		map->_dm_nused = used;
 	}
 	return (error);
 }
@@ -322,7 +326,7 @@ _bus_dmamap_load_uio(bus_dma_tag_t t, bu
     int flags)
 {
 	paddr_t lastaddr = 0;
-	int seg, i, error, first;
+	int seg, used, i, error, first;
 	bus_size_t minlen, resid;
 	struct proc *p = NULL;
 	struct iovec *iov;
@@ -347,6 +351,7 @@ _bus_dmamap_load_uio(bus_dma_tag_t t, bu
 
 	first = 1;
 	seg = 0;
+	used = 0;
 	error = 0;
 	for (i = 0; i < uio->uio_iovcnt && resid != 0 && error == 0; i++) {
 		/*
@@ -357,7 +362,7 @@ _bus_dmamap_load_uio(bus_dma_tag_t t, bu
 		addr = (caddr_t)iov[i].iov_base;
 
 		error = _bus_dmamap_load_buffer(t, map, addr, minlen,
-		    p, flags, &lastaddr, &seg, first);
+		    p, flags, &lastaddr, &seg, &used, first);
 		first = 0;
 
 		resid -= minlen;
@@ -365,6 +370,7 @@ _bus_dmamap_load_uio(bus_dma_tag_t t, bu
 	if (error == 0) {
 		map->dm_mapsize = uio->uio_resid;
 		map->dm_nsegs = seg + 1;
+		map->_dm_nused = used;
 	}
 	return (error);
 }
@@ -683,8 +689,8 @@ _bus_dmamem_mmap(bus_dma_tag_t t, bus_dm
  */
 int
 _bus_dmamap_load_buffer(bus_dma_tag_t t, bus_dmamap_t map, void *buf,
-    bus_size_t buflen, struct proc *p, int flags, paddr_t *lastaddrp, int *segp,
-    int first)
+    bus_size_t buflen, struct proc *p, int flags, paddr_t *lastaddrp,
+    int *segp, int *usedp, int first)
 {
 	bus_size_t sgsize;
 	bus_addr_t curaddr, lastaddr, baddr, bmask;
@@ -699,6 +705,7 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
 	else
 		pmap = pmap_kernel();
 
+	page = *usedp;
 	lastaddr = *lastaddrp;
 	bmask  = ~(map->_dm_boundary - 1);
 
@@ -714,14 +721,14 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
 			    curaddr);
 
 		if (use_bounce_buffer) {
-			if (map->_dm_nused + 1 >= map->_dm_npages)
+			if (page >= map->_dm_npages)
 				return (ENOMEM);
 
 			off = vaddr & PAGE_MASK;
-			pg = map->_dm_pages[page = map->_dm_nused++];
+			pg = map->_dm_pages[page];
 			curaddr = VM_PAGE_TO_PHYS(pg) + off;
-
 			pgva = map->_dm_pgva + (page << PGSHIFT) + off;
+			page++;
 		}
 
 		/*
@@ -774,6 +781,7 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
 	}
 
 	*segp = seg;
+	*usedp = page;
 	*lastaddrp = lastaddr;
 
 	/*