* [PATCH 1/2] iommu/vt-d: Detach domain before using a private one
@ 2019-08-06 0:14 Lu Baolu
2019-08-06 0:14 ` [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain Lu Baolu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Lu Baolu @ 2019-08-06 0:14 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse
Cc: kevin.tian, ashok.raj, linux-kernel, iommu, Alex Williamson,
jacob.jun.pan
When the default domain of a group doesn't work for a device,
the iommu driver will try to use a private domain. The domain
which was previously attached to the device must be detached.
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private")
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Link: https://lkml.org/lkml/2019/8/2/1379
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel-iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3e22fa6ae8c8..37259b7f95a7 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3449,6 +3449,7 @@ static bool iommu_need_mapping(struct device *dev)
dmar_domain = to_dmar_domain(domain);
dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
}
+ dmar_remove_one_dev_info(dev);
get_private_domain_for_dev(dev);
}
@@ -4803,7 +4804,8 @@ static void dmar_remove_one_dev_info(struct device *dev)
spin_lock_irqsave(&device_domain_lock, flags);
info = dev->archdata.iommu;
- __dmar_remove_one_dev_info(info);
+ if (info)
+ __dmar_remove_one_dev_info(info);
spin_unlock_irqrestore(&device_domain_lock, flags);
}
@@ -5281,6 +5283,7 @@ static int intel_iommu_add_device(struct device *dev)
if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
ret = iommu_request_dm_for_dev(dev);
if (ret) {
+ dmar_remove_one_dev_info(dev);
dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
domain_add_dev_info(si_domain, dev);
dev_info(dev,
@@ -5291,6 +5294,7 @@ static int intel_iommu_add_device(struct device *dev)
if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
ret = iommu_request_dma_domain_for_dev(dev);
if (ret) {
+ dmar_remove_one_dev_info(dev);
dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
if (!get_private_domain_for_dev(dev)) {
dev_warn(dev,
--
2.17.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain
2019-08-06 0:14 [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Lu Baolu
@ 2019-08-06 0:14 ` Lu Baolu
2019-08-08 20:10 ` Alex Williamson
2019-08-08 20:10 ` [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Alex Williamson
2019-08-09 15:35 ` Joerg Roedel
2 siblings, 1 reply; 6+ messages in thread
From: Lu Baolu @ 2019-08-06 0:14 UTC (permalink / raw)
To: Joerg Roedel, David Woodhouse
Cc: kevin.tian, ashok.raj, linux-kernel, iommu, Alex Williamson,
jacob.jun.pan
Multiple devices might share a private domain. One real example
is a pci bridge and all devices behind it. When remove a private
domain, make sure that it has been detached from all devices to
avoid use-after-free case.
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private")
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel-iommu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 37259b7f95a7..12d094d08c0a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4791,7 +4791,8 @@ static void __dmar_remove_one_dev_info(struct device_domain_info *info)
/* free the private domain */
if (domain->flags & DOMAIN_FLAG_LOSE_CHILDREN &&
- !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY))
+ !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY) &&
+ list_empty(&domain->devices))
domain_exit(info->domain);
free_devinfo_mem(info);
--
2.17.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: Detach domain before using a private one
2019-08-06 0:14 [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Lu Baolu
2019-08-06 0:14 ` [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain Lu Baolu
@ 2019-08-08 20:10 ` Alex Williamson
2019-08-09 0:48 ` Lu Baolu
2019-08-09 15:35 ` Joerg Roedel
2 siblings, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2019-08-08 20:10 UTC (permalink / raw)
To: Lu Baolu
Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan,
David Woodhouse
On Tue, 6 Aug 2019 08:14:08 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:
> When the default domain of a group doesn't work for a device,
> the iommu driver will try to use a private domain. The domain
> which was previously attached to the device must be detached.
>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Link: https://lkml.org/lkml/2019/8/2/1379
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
Tested-by: Alex Williamson <alex.williamson@redhat.com>
This series resolves the issue I reported against:
iommu/vt-d: Cleanup get_valid_domain_for_dev()
Thanks,
Alex
> drivers/iommu/intel-iommu.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 3e22fa6ae8c8..37259b7f95a7 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3449,6 +3449,7 @@ static bool iommu_need_mapping(struct device *dev)
> dmar_domain = to_dmar_domain(domain);
> dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
> }
> + dmar_remove_one_dev_info(dev);
> get_private_domain_for_dev(dev);
> }
>
> @@ -4803,7 +4804,8 @@ static void dmar_remove_one_dev_info(struct device *dev)
>
> spin_lock_irqsave(&device_domain_lock, flags);
> info = dev->archdata.iommu;
> - __dmar_remove_one_dev_info(info);
> + if (info)
> + __dmar_remove_one_dev_info(info);
> spin_unlock_irqrestore(&device_domain_lock, flags);
> }
>
> @@ -5281,6 +5283,7 @@ static int intel_iommu_add_device(struct device *dev)
> if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
> ret = iommu_request_dm_for_dev(dev);
> if (ret) {
> + dmar_remove_one_dev_info(dev);
> dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
> domain_add_dev_info(si_domain, dev);
> dev_info(dev,
> @@ -5291,6 +5294,7 @@ static int intel_iommu_add_device(struct device *dev)
> if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
> ret = iommu_request_dma_domain_for_dev(dev);
> if (ret) {
> + dmar_remove_one_dev_info(dev);
> dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
> if (!get_private_domain_for_dev(dev)) {
> dev_warn(dev,
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain
2019-08-06 0:14 ` [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain Lu Baolu
@ 2019-08-08 20:10 ` Alex Williamson
0 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2019-08-08 20:10 UTC (permalink / raw)
To: Lu Baolu
Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan,
David Woodhouse
On Tue, 6 Aug 2019 08:14:09 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:
> Multiple devices might share a private domain. One real example
> is a pci bridge and all devices behind it. When remove a private
> domain, make sure that it has been detached from all devices to
> avoid use-after-free case.
>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
Tested-by: Alex Williamson <alex.williamson@redhat.com>
> drivers/iommu/intel-iommu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 37259b7f95a7..12d094d08c0a 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4791,7 +4791,8 @@ static void __dmar_remove_one_dev_info(struct device_domain_info *info)
>
> /* free the private domain */
> if (domain->flags & DOMAIN_FLAG_LOSE_CHILDREN &&
> - !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY))
> + !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY) &&
> + list_empty(&domain->devices))
> domain_exit(info->domain);
>
> free_devinfo_mem(info);
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: Detach domain before using a private one
2019-08-08 20:10 ` [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Alex Williamson
@ 2019-08-09 0:48 ` Lu Baolu
0 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2019-08-09 0:48 UTC (permalink / raw)
To: Alex Williamson
Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan,
David Woodhouse
Hi Alex,
On 8/9/19 4:10 AM, Alex Williamson wrote:
> On Tue, 6 Aug 2019 08:14:08 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
>> When the default domain of a group doesn't work for a device,
>> the iommu driver will try to use a private domain. The domain
>> which was previously attached to the device must be detached.
>>
>> Cc: Ashok Raj <ashok.raj@intel.com>
>> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private")
>> Reported-by: Alex Williamson <alex.williamson@redhat.com>
>> Link: https://lkml.org/lkml/2019/8/2/1379
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>
> Tested-by: Alex Williamson <alex.williamson@redhat.com>
>
> This series resolves the issue I reported against:
> iommu/vt-d: Cleanup get_valid_domain_for_dev()
Thank you for testing it.
Best regards,
Baolu
>
> Thanks,
> Alex
>
>> drivers/iommu/intel-iommu.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 3e22fa6ae8c8..37259b7f95a7 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -3449,6 +3449,7 @@ static bool iommu_need_mapping(struct device *dev)
>> dmar_domain = to_dmar_domain(domain);
>> dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
>> }
>> + dmar_remove_one_dev_info(dev);
>> get_private_domain_for_dev(dev);
>> }
>>
>> @@ -4803,7 +4804,8 @@ static void dmar_remove_one_dev_info(struct device *dev)
>>
>> spin_lock_irqsave(&device_domain_lock, flags);
>> info = dev->archdata.iommu;
>> - __dmar_remove_one_dev_info(info);
>> + if (info)
>> + __dmar_remove_one_dev_info(info);
>> spin_unlock_irqrestore(&device_domain_lock, flags);
>> }
>>
>> @@ -5281,6 +5283,7 @@ static int intel_iommu_add_device(struct device *dev)
>> if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
>> ret = iommu_request_dm_for_dev(dev);
>> if (ret) {
>> + dmar_remove_one_dev_info(dev);
>> dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
>> domain_add_dev_info(si_domain, dev);
>> dev_info(dev,
>> @@ -5291,6 +5294,7 @@ static int intel_iommu_add_device(struct device *dev)
>> if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
>> ret = iommu_request_dma_domain_for_dev(dev);
>> if (ret) {
>> + dmar_remove_one_dev_info(dev);
>> dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
>> if (!get_private_domain_for_dev(dev)) {
>> dev_warn(dev,
>
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: Detach domain before using a private one
2019-08-06 0:14 [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Lu Baolu
2019-08-06 0:14 ` [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain Lu Baolu
2019-08-08 20:10 ` [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Alex Williamson
@ 2019-08-09 15:35 ` Joerg Roedel
2 siblings, 0 replies; 6+ messages in thread
From: Joerg Roedel @ 2019-08-09 15:35 UTC (permalink / raw)
To: Lu Baolu
Cc: kevin.tian, ashok.raj, iommu, linux-kernel, Alex Williamson,
jacob.jun.pan, David Woodhouse
On Tue, Aug 06, 2019 at 08:14:08AM +0800, Lu Baolu wrote:
> drivers/iommu/intel-iommu.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Applied to iommu/fixes, thanks.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-09 15:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 0:14 [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Lu Baolu
2019-08-06 0:14 ` [PATCH 2/2] iommu/vt-d: Fix possible use-after-free of private domain Lu Baolu
2019-08-08 20:10 ` Alex Williamson
2019-08-08 20:10 ` [PATCH 1/2] iommu/vt-d: Detach domain before using a private one Alex Williamson
2019-08-09 0:48 ` Lu Baolu
2019-08-09 15:35 ` Joerg Roedel
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).