Messages in this thread | | | Date | Thu, 27 Jan 2022 18:42:00 -0800 | From | Fenghua Yu <> | Subject | Re: [PATCH v2 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit |
| |
Hi, Thomas,
On Wed, Jan 26, 2022 at 10:38:04PM +0100, Thomas Gleixner wrote: > On Wed, Jan 26 2022 at 09:36, Fenghua Yu wrote: > > On Wed, Jan 26, 2022 at 03:23:42PM +0100, Thomas Gleixner wrote: > >> On Tue, Jan 25 2022 at 07:18, Fenghua Yu wrote: > >> While looking at ioasid_put() usage I tripped over the following UAF > >> issue: > >> > >> --- a/drivers/iommu/intel/iommu.c > >> +++ b/drivers/iommu/intel/iommu.c > >> @@ -4817,8 +4817,10 @@ static int aux_domain_add_dev(struct dma > >> auxiliary_unlink_device(domain, dev); > >> link_failed: > >> spin_unlock_irqrestore(&device_domain_lock, flags); > >> - if (list_empty(&domain->subdevices) && domain->default_pasid > 0) > >> + if (list_empty(&domain->subdevices) && domain->default_pasid > 0) { > >> ioasid_put(domain->default_pasid); > >> + domain->default_pasid = INVALID_IOASID; > >> + } > >> > >> return ret; > >> } > >> @@ -4847,8 +4849,10 @@ static void aux_domain_remove_dev(struct > >> > >> spin_unlock_irqrestore(&device_domain_lock, flags); > >> > >> - if (list_empty(&domain->subdevices) && domain->default_pasid > 0) > >> + if (list_empty(&domain->subdevices) && domain->default_pasid > 0) { > >> ioasid_put(domain->default_pasid); > >> + domain->default_pasid = INVALID_IOASID; > >> + } > >> } > >> > >> static int prepare_domain_attach_device(struct iommu_domain *domain, > > > > The above patch fixes an existing issue. I will put it in a separate patch, > > right? > > Correct. > > > It cannot be applied cleanly to the upstream tree. Do you want me to base > > the above patch (and the whole patch set) to the upstream tree or a specific > > tip branch? > > Against Linus tree please so that the bugfix applies. > > > I will fold the following patch into patch #5. The patch #11 (the doc patch) > > also needs to remove one paragraph talking about refcount. > > > > So I will send the whole patch set with the following changes: > > 1. One new bug fix patch (the above patch)
When I study your above aux_domain bug fix path, I find more aux_domain bugs. But then I find aux_domain will be completely removed because all aux_domain related callbacks are not called and are dead code (no wonder there are so many bugs in aux_domain). Please see this series: https://lore.kernel.org/linux-iommu/20220124071103.2097118-4-baolu.lu@linux.intel.com/ For the series, Baolu confirms that he is "pretty sure that should be part of v5.18". And I don't find the series calls any IOASID function after removing the aux_domain code.
So that means we don't need to fix those issues in the dead aux_domain code any more because it will be completely removed in 5.18, right?
If you agree, I will not include the aux_domain fix patch or any other aux_domain fix patches in the up-coming v3. Is that OK?
> > 2. Updated patch #5 (with the following patch folded)
I will still change ioasid_put() in the aux_domain callbacks to ioasid_free() in patch #5. So compilation will pass. Baolu's series will remove the entire aux_domain code in 5.18.
> > 3. Updated patch #11 (removing refcount description) > > Looks good. >
Thanks.
-Fenghua
| |