Date: Thu, 14 Jun 2018 14:29:02 +0530 From: Pratyush Yadav <pratiy0100@gmail.com> To: freebsd-hackers@freebsd.org Cc: Edward Napierala <trasz@freebsd.org>, =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= <royger@freebsd.org> Subject: Bug in subr_bus_dma.c's bus_dmamap_load() when multiple dma maps are created from a single tag on x86 Message-ID: <CA%2BX=3TL1yeLvwPFqt0GAG7qxKC3Gv9mWzdY84=WD%2BSeLRihv1A@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
Hi everyone, I was looking at the busdma code for x86 and I notice that the claim in the bus_dma(9) man page might not be correct: "Multiple maps can be associated with one DMA tag." Looking at the code of sys/x86/x86/busdma_bounce.c (which I believe is the default busdma implementation for x86), I realize that a map does not necessarily use all the segments of the dma tag (specified by the parameter nsegments on tag creation). So how many segments does a map actually use? Looking at busdma_bounce.c:644,690 it seems we can calculate that from the pointer segp. It contains the index of starting segment on entrace to the load function, and the ending segment on exit. Sounds all fine so far. But then if we look at map_complete() (busdma_bounce.c:908), it returns the entire segments array of the dma *tag*. I emphasize tag because the tag's segments array holds the segments of all the maps created with the tag. Going a layer up, sys/kern/subr_bus_dma.c:328 simply passes nsegs(= -1) in the segp parameter to load, not bothering to check if some segments have been used already by other maps. It then calculates the number of segments used using the value of nsegs after return and calls map_complete() to get the segments array. It then passes the segments array to the driver callback with the number of segments, nsegs. Effectively, to the driver it seems that the map's segments start from index 0 and go till nsegs - 1. This is not true if another map has been loaded in the meantime with the same tag. We are possibly over-writing the previous map's segments. busdma_bounce.c:669 simply uses segs[0] if the value of segp is -1, which is what bus_dmamap_load() in subr_bus_dma.c:328 passes to _bus_dmamap_load_buffer(). segs[0] would also be used by any subsequent map's load. This means the previous map's values are over-written. This might cause problems when the driver is in its callback using the segments array and another map is loaded. Unfortunately, I do not posses the knowledge or experience with FreeBSD's drivers (drivers in general) to test for this bug. I have not looked at the code in other architectures. The same problem might present itself there as well. If I made a mistake somewhere in my reasoning, let me know. -- Regards, Pratyush Yadav
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BX=3TL1yeLvwPFqt0GAG7qxKC3Gv9mWzdY84=WD%2BSeLRihv1A>