[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [for-4.15][PATCH v2 4/5] xen/iommu: x86: Don't leak the IOMMU page-tables
On 10.02.2021 16:04, Julien Grall wrote: > On 10/02/2021 14:32, Jan Beulich wrote: >> On 09.02.2021 16:28, Julien Grall wrote: >>> From: Julien Grall <jgrall@xxxxxxxxxx> >>> >>> The new IOMMU page-tables allocator will release the pages when >>> relinquish the domain resources. However, this is not sufficient when >>> the domain is dying because nothing prevents page-table to be allocated. >>> >>> iommu_alloc_pgtable() is now checking if the domain is dying before >>> adding the page in the list. We are relying on &hd->arch.pgtables.lock >>> to synchronize d->is_dying. >> >> As said in reply to an earlier patch, I think suppressing >> (really: ignoring) new mappings would be better. > > This is exactly what I suggested in v1 but you wrote: > > "Ignoring requests there seems fragile to me. Paul - what are your > thoughts about bailing early from hvm_add_ioreq_gfn() when the > domain is dying?" Was this on the thread of this patch? I didn't find such a reply of mine. I need more context here because you name hvm_add_ioreq_gfn() above, while I refer to iommu_map() (and downwards the call stack). > Are you know saying that the following snipped would be fine: > > if ( d->is_dying ) > return 0; In {amd,intel}_iommu_map_page(), after the lock was acquired and with it suitably released, yes. And if that's what you suggested, then I'm sorry - I don't think I can see anything fragile there anymore. >> You could >> utilize the same lock, but you'd need to duplicate the >> checking in {amd,intel}_iommu_map_page(). >> >> I'm not entirely certain in the case about unmap requests: >> It may be possible to also suppress/ignore them, but this >> may require some further thought. > > I think the unmap part is quite risky to d->is_dying because the PCI > devices may not quiesced and still assigned to the domain. Hmm, yes, good point. Of course upon first unmap with is_dying observed set we could zap the root page table, but I don't suppose that's something we want to do for 4.15. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |