On Fri, Nov 20, 2020 at 10:11:58AM +0800, Lu Baolu wrote:
> On 11/19/20 4:53 PM, Will Deacon wrote:
> > On Thu, Nov 19, 2020 at 10:18:05AM +0800, Lu Baolu wrote:
> > > On 11/18/20 9:51 PM, Will Deacon wrote:
> > > > On Fri, Sep 25, 2020 at 12:06:18PM -0700, Ashok Raj wrote:
> > > > > From: Sai Praneeth Prakhya <[email protected]>
> > 
> > [...]
> > 
> > > > > +free_new_domain:
> > > > > +     iommu_domain_free(group->default_domain);
> > > > > +     group->default_domain = prev_dom;
> > > > > +     group->domain = prev_dom;i
> > > > 
> > > > Hmm. This seems to rely on all users of group->default_domain holding 
> > > > the
> > > > group->mutex. Have you confirmed that this is the case? There's a funny
> > > > use of iommu_group_get() in the exynos IOMMU driver at least.
> > > 
> > > Emm. This change happens within the area with group->mutex held. Or I
> > > am not getting your point?
> > 
> > Yeah, sorry, I wasn't very clear. This code holds the group->mutex, and it
> > relies on _anybody_ else who wants to inspect group->default_domain also
> > holding that mutex, otherwise they could observe a transient domain pointer
> > which we free on the failure path here.
> 
> Clear to me now. Thanks for explanation. :-)
> 
> Changing default domain through sysfs requires the users to ubind any
> driver from the devices in the group. There's a check code and return
> failure if this requirement doesn't meet.
> 
> So we only need to consider the device release path. device_lock(dev) is
> used in this patch to guarantee that no device release happens at the
> same time.

Aha, thanks. Please can you add a comment for future reference?

> 
> > 
> > My question is whether or not there is code that inspects
> > group->default_domain without group->mutex held? The exynos case doesn't
> > obviously hold it, and I'd like to make sure that there aren't others that
> > we need to worry about.
> 
> I searched the code. The exynos is the only case that inspects
> group->default_domain without holding the mutex during run time. It's in
> the device release path, so I think it's safe.

Great, thanks for looking.

Will
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to