From ee755665c268aa5b22d7c4fd40dc02dfac64bfab Mon Sep 17 00:00:00 2001 From: Bill Paul Date: Tue, 14 Aug 2001 21:56:53 +0000 Subject: [PATCH] Fix yet another "unconditionally free()ing memory without even checking to see if it was malloc()ed first" bug. In bus_dmamap_create(), one of two things can happen: either we need to allocate a special map due to some manner of bounce buffering requirement, or we can DMA a buffer in place. On the x86 platform, the "in place" case results in bus_dmamap_create() returning a dmamap of NULL. The bus_dmamap_destroy() routine later checks for NULL and won't bother free()ing the map if it detects this condition. But on the alpha, we don't use NULL, we use a statically allocated map called nobounce_dmamap(). Unfortunately, bus_dmamap_destroy() does not handle the condition where we attempt to destroy such a map: it tries to free() the dmamap, which causes a panic. Fix: test that map != &nobounce_dmamap before trying to free() it. With this fix, my busdma-ified if_sis driver works on the alpha. I'm a bit alarmed that I'm the first person ever to trip over this bug, since we have been using busdma on the alpha for a while, and since it sort of screams out "Hi! I'm a bug! Booga-booga!" when you look at it. (Somewhere, somebody will say: "But Bill, why don't you just not bother destroying the maps in this case." Because the API is supposed to be a) symetrical and b) opaque to the caller. I can't know whether it's safe to skip the bus_dmamap_destroy() step or not without sticking my fingers into unsafe places, which is what I wanted to avoid in the first place.) --- sys/alpha/alpha/busdma_machdep.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sys/alpha/alpha/busdma_machdep.c b/sys/alpha/alpha/busdma_machdep.c index bed5e8a9290..ea984c35e1a 100644 --- a/sys/alpha/alpha/busdma_machdep.c +++ b/sys/alpha/alpha/busdma_machdep.c @@ -327,7 +327,13 @@ bus_dmamap_destroy(bus_dma_tag_t dmat, bus_dmamap_t map) if (map != NULL) { if (STAILQ_FIRST(&map->bpages) != NULL) return (EBUSY); - free(map, M_DEVBUF); + /* + * The nobounce_dmamap map is not dynamically + * allocated, thus we should on no account try to + * free it. + */ + if (map != &nobounce_dmamap) + free(map, M_DEVBUF); } dmat->map_count--; return (0);