xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: paul@xen.org
Cc: hongyxia@amazon.co.uk, iwj@xenproject.org,
	Julien Grall <jgrall@amazon.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Kevin Tian <kevin.tian@intel.com>,
	xen-devel@lists.xenproject.org, Julien Grall <julien@xen.org>
Subject: Re: [for-4.15][PATCH v3 2/3] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying
Date: Fri, 19 Feb 2021 09:56:32 +0100	[thread overview]
Message-ID: <d818154a-2176-2607-b893-6e77e57e5c82@suse.com> (raw)
In-Reply-To: <14f21eac-7d5d-9dda-18d2-206614e91339@xen.org>

On 18.02.2021 15:00, Paul Durrant wrote:
> On 18/02/2021 13:05, Jan Beulich wrote:
>> On 17.02.2021 17:07, Julien Grall wrote:
>>> On 17/02/2021 15:01, Jan Beulich wrote:
>>>> On 17.02.2021 15:24, Julien Grall wrote:
>>>>> From: Julien Grall <jgrall@amazon.com>
>>>>> The new x86 IOMMU page-tables allocator will release the pages when
>>>>> relinquishing the domain resources. However, this is not sufficient
>>>>> when the domain is dying because nothing prevents page-table to be
>>>>> allocated.
>>>>> Currently page-table allocations can only happen from iommu_map(). As
>>>>> the domain is dying, there is no good reason to continue to modify the
>>>>> IOMMU page-tables.
>>>> While I agree this to be the case right now, I'm not sure it is a
>>>> good idea to build on it (in that you leave the unmap paths
>>>> untouched).
>>> I don't build on that assumption. See next patch.
>> Yet as said there that patch makes unmapping perhaps more fragile,
>> by introducing a latent error source into the path.
>>>> Imo there's a fair chance this would be overlooked at
>>>> the point super page mappings get introduced (which has been long
>>>> overdue), and I thought prior discussion had lead to a possible
>>>> approach without risking use-after-free due to squashed unmap
>>>> requests.
>>> I know you suggested to zap the root page-tables... However, I don't
>>> think this is 4.15 material and you agree with this (you were the one
>>> pointed out that out).
>> Paul - do you have any thoughts here? Outright zapping isn't
>> going to work, we'd need to switch to quarantine page tables at
>> the very least to prevent the issue with babbling devices. But
>> that still seems better to me than the introduction of a latent
>> issue on the unmap paths.
> I've not really been following this. AFAICT clear_root_pgtable() does 
> not actually mess with any context entries so the device view of the 
> tables won't change, will it?

Correct. Elsewhere I said that "zapping" here has two meanings,
one is what Julien does and the other is what I mean and what I
understand you also refer to - to actually replace the mappings
referenced by a context entry as soon as we no longer guarantee
to update them correctly.

My concern with not touching the unmap paths is that if there
was any "best effort" unmap left anywhere in the tree, we may
still end up with a use-after-free when late unmaps are now
made possibly fail by failing late allocation attempts.


  reply	other threads:[~2021-02-19  8:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-17 14:24 [for-4.15][PATCH v3 0/3] xen/iommu: Collection of bug fixes for IOMMU teadorwn Julien Grall
2021-02-17 14:24 ` [for-4.15][PATCH v3 1/3] xen/iommu: x86: Clear the root page-table before freeing the page-tables Julien Grall
2021-02-17 14:54   ` Jan Beulich
2021-02-17 15:00     ` Julien Grall
2021-02-17 15:17       ` Jan Beulich
2021-02-17 16:48         ` Julien Grall
2021-02-17 14:24 ` [for-4.15][PATCH v3 2/3] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying Julien Grall
2021-02-17 15:01   ` Jan Beulich
2021-02-17 16:07     ` Julien Grall
2021-02-18 13:05       ` Jan Beulich
2021-02-18 13:25         ` Julien Grall
2021-02-19  8:49           ` Jan Beulich
2021-02-19  9:24             ` Julien Grall
2021-02-18 14:00         ` Paul Durrant
2021-02-19  8:56           ` Jan Beulich [this message]
2021-02-17 14:24 ` [for-4.15][PATCH v3 3/3] xen/iommu: x86: Harden the IOMMU page-table allocator Julien Grall
2021-02-17 15:13   ` Jan Beulich
2021-02-17 16:29     ` Julien Grall
2021-02-18 13:10       ` Jan Beulich
2021-02-18 13:19         ` Julien Grall
2021-02-18 17:04           ` Jan Beulich
2021-02-18 17:41             ` Julien Grall
2021-02-19  8:46               ` Jan Beulich
2021-02-19  8:57                 ` Julien Grall

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:

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

  git send-email \
    --in-reply-to=d818154a-2176-2607-b893-6e77e57e5c82@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=hongyxia@amazon.co.uk \
    --cc=iwj@xenproject.org \
    --cc=jgrall@amazon.com \
    --cc=julien@xen.org \
    --cc=kevin.tian@intel.com \
    --cc=paul@xen.org \
    --cc=xen-devel@lists.xenproject.org \


* 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;
as well as URLs for NNTP newsgroup(s).