Linux IOMMU Development
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Lu Baolu <baolu.lu@linux.intel.com>
Cc: Will Deacon <will.deacon@arm.com>,
	iommu@lists.linux-foundation.org,
	Robin Murphy <robin.murphy@arm.com>,
	Ashok Raj <ashok.raj@intel.com>, Christoph Hellwig <hch@lst.de>
Subject: Re: [Patch V8 1/3] iommu: Add support to change default domain of an iommu group
Date: Thu, 19 Nov 2020 08:53:04 +0000	[thread overview]
Message-ID: <20201119085303.GA3599@willie-the-truck> (raw)
In-Reply-To: <568d4dee-ad10-ca4f-d51a-8909afb6b782@linux.intel.com>

On Thu, Nov 19, 2020 at 10:18:05AM +0800, Lu Baolu wrote:
> The original author of this patch series has left Intel. I am now the
> backup.

Ok, thanks for letting me know.

> 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 <sai.praneeth.prakhya@intel.com>

[...]

> > > +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.

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.

Does that make more sense?

Thanks,

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2020-11-19  8:53 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 19:06 [Patch V8 0/3] iommu: Add support to change default domain of an iommu group Ashok Raj
2020-09-25 19:06 ` [Patch V8 1/3] " Ashok Raj
2020-11-18 13:51   ` Will Deacon
2020-11-19  2:18     ` Lu Baolu
2020-11-19  8:53       ` Will Deacon [this message]
2020-11-20  2:11         ` Lu Baolu
2020-11-20 11:03           ` Will Deacon
2020-11-20 11:27   ` Shameerali Kolothum Thodi
2020-11-20 13:09     ` Lu Baolu
2020-09-25 19:06 ` [Patch V8 2/3] iommu: Take lock before reading iommu group default domain type Ashok Raj
2020-09-25 19:06 ` [Patch V8 3/3] iommu: Document usage of "/sys/kernel/iommu_groups/<grp_id>/type" file Ashok Raj
2020-11-18 13:51   ` Will Deacon
2020-11-19  2:32     ` Lu Baolu
2020-11-19  8:55       ` Will Deacon
2020-11-20  2:13         ` Lu Baolu
2020-10-01 12:58 ` [Patch V8 0/3] iommu: Add support to change default domain of an iommu group Joerg Roedel
2020-10-01 13:51   ` Raj, Ashok
2020-11-18 13:52 ` Will Deacon
2020-11-19  2:36   ` Lu Baolu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201119085303.GA3599@willie-the-truck \
    --to=will@kernel.org \
    --cc=ashok.raj@intel.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux-foundation.org \
    --cc=robin.murphy@arm.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox