On Thu, Aug 15, 2024 at 01:21:01PM -0400, Peter Xu wrote: > > Why? Either the function only returns PFN map no-struct page things or > > it returns struct page stuff too, in which case why bother to check > > the VMA flags if the caller already has to be correct for struct page > > backed results? > > > > This function is only safe to use under the proper locking, and under > > those rules it doesn't matter at all what the result is.. > > Do you mean we should drop the PFNMAP|IO check? Yeah > I didn't see all the > callers to say that they won't rely on proper failing of !PFNMAP&&!IO vmas > to work alright. So I assume we should definitely keep them around. But as before, if we care about this we should be using vm_normal_page as that is sort of abusing the PFNMAP flags. > > Any physical address obtained through this API is only valid while > > the @follow_pfnmap_args. Continuing to use the address after end(), > > without some other means to synchronize with page table updates > > will create a security bug. > > Some misuse on wordings here (e.g. we don't return PA but PFN), and some > sentence doesn't seem to be complete.. but I think I get the "scary" part > of it. How about this, appending the scary part to the end? > > * During the start() and end() calls, the results in @args will be valid > * as proper locks will be held. After the end() is called, all the fields > * in @follow_pfnmap_args will be invalid to be further accessed. Further > * use of such information after end() may require proper synchronizations > * by the caller with page table updates, otherwise it can create a > * security bug. I would specifically emphasis that the pfn may not be used after end. That is the primary mistake people have made. They think it is a PFN so it is safe. > It sounds like we need some mmu notifiers when mapping the IOMMU pgtables, > as long as there's MMIO-region / P2P involved. It'll make sure when > tearing down the BAR mappings, the devices will at least see the same view > as the processors. I think the mmu notifiers can trigger too often for this to be practical for DMA :( Jason
