On Mon, May 16, 2016 at 12:53:54AM +0200, Mark Kettenis wrote: > So Theo's SROP mitigation diff made my armv7 machine completely > unstable. Filesystem-related panics, random core dumps, files with > blocks of zeroes on them, etc. > > Looking at the changes in question, there is no way they're > responsible for this behavior. This smells like cache-related bug > triggered by changes in the memory layout in the kernel. Since I'm > running with root on an SD card hanging off imxesdhc(4), I started > looking for missing bus_dmamap_sync() calls in that driver. Found > one, but notone that actually mattered. Then I turned to the actual > bus_dmamap_sync(9) implementation. It has a nice comment explaining > when cache maintenance is needed. It mentions that the D-Cache needs > to be invalidated for a POSTREAD operation. But it doesn't actually > do that. > > The NetBSD code (from which our code is derived) does implement this, > and has an additional comment: > > /* > * Cortex CPUs can do speculative loads so we need to clean the cache > * after a DMA read to deal with any speculatively loaded cache lines. > * Since these can't be dirty, we can just invalidate them and don't > * have to worry about having to write back their contents. > */ > > Adding the relevant bit of code seems to fix the issue. > > So here's the diff. It seems that this code isn't needed on the > pre-ARMv7 hardware. So I could sprinkle in an #ifdef CPU_ARMv7 if > people think that is worth it. But that might make us forget this > code for future ARM processor generations. > > ok?
ok jsg@ > > > Index: arch/arm/arm/bus_dma.c > =================================================================== > RCS file: /cvs/src/sys/arch/arm/arm/bus_dma.c,v > retrieving revision 1.29 > diff -u -p -r1.29 bus_dma.c > --- arch/arm/arm/bus_dma.c 10 Mar 2016 10:22:43 -0000 1.29 > +++ arch/arm/arm/bus_dma.c 15 May 2016 22:50:22 -0000 > @@ -421,6 +421,19 @@ _bus_dmamap_sync_segment(vaddr_t va, pad > cpu_dcache_wb_range(va, len); > cpu_sdcache_wb_range(va, pa, len); > break; > + > + /* > + * Cortex CPUs can do speculative loads so we need to clean the cache > + * after a DMA read to deal with any speculatively loaded cache lines. > + * Since these can't be dirty, we can just invalidate them and don't > + * have to worry about having to write back their contents. > + */ > + case BUS_DMASYNC_POSTREAD: > + case BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE: > + membar_sync(); > + cpu_dcache_inv_range(va, len); > + cpu_sdcache_inv_range(va, pa, len); > + break; > } > } > >
