| Message ID | 20180302101050.6191-2-vivek.gautam@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On 02/03/18 10:10, Vivek Gautam wrote: > If we fail after initializing domain_context, we should destroy > the context to free up resources. Have another think about why the "problem" this patch caters for cannot ever happen (hint: consider how domain->smmu is used in arm_smmu_init_domain_context()). And then also about the really catastrophically bad problem it actually introduces (hint: "iommu_attach(domain, good_dev); iommu_attach(domain, bad_dev);") Robin. > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> > --- > > * New patch added in this series. > > drivers/iommu/arm-smmu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 69e7c60792a8..ffc152c36002 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -1223,11 +1223,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) > dev_err(dev, > "cannot attach to SMMU %s whilst already attached to domain on SMMU %s\n", > dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev)); > - return -EINVAL; > + ret = -EINVAL; > + goto destroy_domain; > } > > /* Looks ok, so add the device to the domain */ > return arm_smmu_domain_add_master(smmu_domain, fwspec); > + > +destroy_domain: > + arm_smmu_destroy_domain_context(domain); > + return ret; > } > > static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova, > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 7, 2018 at 5:50 PM, Robin Murphy <robin.murphy@arm.com> wrote: > On 02/03/18 10:10, Vivek Gautam wrote: >> >> If we fail after initializing domain_context, we should destroy >> the context to free up resources. > > > Have another think about why the "problem" this patch caters for cannot ever > happen (hint: consider how domain->smmu is used in > arm_smmu_init_domain_context()). And then also about the really > catastrophically bad problem it actually introduces (hint: > "iommu_attach(domain, good_dev); iommu_attach(domain, bad_dev);") Got it, we would end up destroying good_dev's domain context with this. Thanks regards Vivek > > Robin. > > >> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> >> --- >> >> * New patch added in this series. >> >> drivers/iommu/arm-smmu.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index 69e7c60792a8..ffc152c36002 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -1223,11 +1223,16 @@ static int arm_smmu_attach_dev(struct iommu_domain >> *domain, struct device *dev) >> dev_err(dev, >> "cannot attach to SMMU %s whilst already attached >> to domain on SMMU %s\n", >> dev_name(smmu_domain->smmu->dev), >> dev_name(smmu->dev)); >> - return -EINVAL; >> + ret = -EINVAL; >> + goto destroy_domain; >> } >> /* Looks ok, so add the device to the domain */ >> return arm_smmu_domain_add_master(smmu_domain, fwspec); >> + >> +destroy_domain: >> + arm_smmu_destroy_domain_context(domain); >> + return ret; >> } >> static int arm_smmu_map(struct iommu_domain *domain, unsigned long >> iova, >> >
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 69e7c60792a8..ffc152c36002 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1223,11 +1223,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) dev_err(dev, "cannot attach to SMMU %s whilst already attached to domain on SMMU %s\n", dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev)); - return -EINVAL; + ret = -EINVAL; + goto destroy_domain; } /* Looks ok, so add the device to the domain */ return arm_smmu_domain_add_master(smmu_domain, fwspec); + +destroy_domain: + arm_smmu_destroy_domain_context(domain); + return ret; } static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
If we fail after initializing domain_context, we should destroy the context to free up resources. Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> --- * New patch added in this series. drivers/iommu/arm-smmu.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)