On Sat, May 6, 2023, at 09:25, Guo Ren wrote: > On Fri, May 5, 2023 at 9:19 PM Arnd Bergmann <arnd@xxxxxxxx> wrote: >> >> This is something we can consider. Unfortunately, this is something >> that no architecture (except pa-risc, which has other problems) >> does at the moment, so we'd probably need to have a proper debate >> about this. >> >> We already have two conflicting ways to handle DMA_FROM_DEVICE, >> either invalidate/invalidate, or clean/invalidate. I can see > I vote to invalidate/invalidate. > ... > >> that flush/invalidate may be a sensible option as well, but I'd >> want to have that discussion after the series is complete, so >> we can come to a generic solution that has the same documented >> behavior across all architectures. > Yes, I agree to unify them into a generic solution first. My proposal > could be another topic in the future. Right, I was explicitly trying to exclude that question from my series, and left it as an architecture specific Kconfig option based on the current behavior. >> In particular, if we end up moving arm64 and riscv back to the >> traditional invalidate/invalidate for DMA_FROM_DEVICE and >> document that driver must not rely on buffers getting cleaned > After invalidation, the cache lines are also cleaned, right? So why do > we need to document it additionally? I mentioned the debate in the cover letter, the full explanation is archived at https://lore.kernel.org/all/20220606152150.GA31568@willie-the-truck/ In short, the problem that is addressed here is leaking sensitive kernel data to user space or a device as in this sequence: 1. A DMA buffer is allocated in the kernel and contains stale data that is no longer needed but must not be exposed to untrusted userspace, i.e. encryption keys or user file pages 2. allocator uses memset() to clear out the buffer 3. buffer gets mapped into a device for DMA_FROM_DEVICE 4. writeback cache gets invalidated, uncovering the sensitive data by discarding the zeros 5. device returns less data than expected 6. buffer is unmapped 7. whole buffer is mapped or copied to user space Will added his patch for arm64 to prevent this scenario by using 'clean' instead of 'invalidate' in step 4, and the same behavior got copied to riscv but not most of the other architectures. The dma-mapping documentation does not say anything about this case, and an alternative approach would be to document that device drivers must watch out for short reads in step 5, or that kzalloc() should clean the cache in step 2. Both of these come at a cost as well. Arnd
