All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] iommu/tegra-smmu: Reference count fixes
@ 2020-08-06 15:54 ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Jon Hunter, Krishna Reddy, iommu, linux-tegra

From: Thierry Reding <treding@nvidia.com>

I was running into some use-after-free conditions after recent changes
to the host1x driver cause the child devices to be destroyed upon driver
unloading. This in turn caused the IOMMU groups associated with the
child devices to also get released and that uncovered a subtle reference
count unbalance.

This contains two fixes for these issues and also includes a patch that
sets the IOMMU group name for "static" groups to help with debugging.

Thierry

Thierry Reding (3):
  iommu/tegra-smmu: Set IOMMU group name
  iommu/tegra-smmu: Balance IOMMU group reference count
  iommu/tegra-smmu: Prune IOMMU group when it is released

 drivers/iommu/tegra-smmu.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 0/3] iommu/tegra-smmu: Reference count fixes
@ 2020-08-06 15:54 ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-tegra, iommu, Jon Hunter

From: Thierry Reding <treding@nvidia.com>

I was running into some use-after-free conditions after recent changes
to the host1x driver cause the child devices to be destroyed upon driver
unloading. This in turn caused the IOMMU groups associated with the
child devices to also get released and that uncovered a subtle reference
count unbalance.

This contains two fixes for these issues and also includes a patch that
sets the IOMMU group name for "static" groups to help with debugging.

Thierry

Thierry Reding (3):
  iommu/tegra-smmu: Set IOMMU group name
  iommu/tegra-smmu: Balance IOMMU group reference count
  iommu/tegra-smmu: Prune IOMMU group when it is released

 drivers/iommu/tegra-smmu.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

-- 
2.27.0

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] iommu/tegra-smmu: Set IOMMU group name
  2020-08-06 15:54 ` Thierry Reding
@ 2020-08-06 15:54   ` Thierry Reding
  -1 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Jon Hunter, Krishna Reddy, iommu, linux-tegra

From: Thierry Reding <treding@nvidia.com>

Set the name of static IOMMU groups to help with debugging.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 124c8848ab7e..1ffdafe892d9 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -847,6 +847,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 		return NULL;
 	}
 
+	iommu_group_set_name(group->group, soc->name);
 	list_add_tail(&group->list, &smmu->groups);
 	mutex_unlock(&smmu->lock);
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/3] iommu/tegra-smmu: Set IOMMU group name
@ 2020-08-06 15:54   ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-tegra, iommu, Jon Hunter

From: Thierry Reding <treding@nvidia.com>

Set the name of static IOMMU groups to help with debugging.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 124c8848ab7e..1ffdafe892d9 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -847,6 +847,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 		return NULL;
 	}
 
+	iommu_group_set_name(group->group, soc->name);
 	list_add_tail(&group->list, &smmu->groups);
 	mutex_unlock(&smmu->lock);
 
-- 
2.27.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] iommu/tegra-smmu: Balance IOMMU group reference count
  2020-08-06 15:54 ` Thierry Reding
@ 2020-08-06 15:54   ` Thierry Reding
  -1 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Jon Hunter, Krishna Reddy, iommu, linux-tegra

From: Thierry Reding <treding@nvidia.com>

For groups that are shared between multiple devices, care must be taken
to acquire a reference for each device, otherwise the IOMMU core ends up
dropping the last reference too early, which will cause the group to be
released while consumers may still be thinking that they're holding a
reference to it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 1ffdafe892d9..c439c0929ef8 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -818,6 +818,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 {
 	const struct tegra_smmu_group_soc *soc;
 	struct tegra_smmu_group *group;
+	struct iommu_group *grp;
 
 	soc = tegra_smmu_find_group(smmu, swgroup);
 	if (!soc)
@@ -827,8 +828,9 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 
 	list_for_each_entry(group, &smmu->groups, list)
 		if (group->soc == soc) {
+			grp = iommu_group_ref_get(group->group);
 			mutex_unlock(&smmu->lock);
-			return group->group;
+			return grp;
 		}
 
 	group = devm_kzalloc(smmu->dev, sizeof(*group), GFP_KERNEL);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] iommu/tegra-smmu: Balance IOMMU group reference count
@ 2020-08-06 15:54   ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-tegra, iommu, Jon Hunter

From: Thierry Reding <treding@nvidia.com>

For groups that are shared between multiple devices, care must be taken
to acquire a reference for each device, otherwise the IOMMU core ends up
dropping the last reference too early, which will cause the group to be
released while consumers may still be thinking that they're holding a
reference to it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 1ffdafe892d9..c439c0929ef8 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -818,6 +818,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 {
 	const struct tegra_smmu_group_soc *soc;
 	struct tegra_smmu_group *group;
+	struct iommu_group *grp;
 
 	soc = tegra_smmu_find_group(smmu, swgroup);
 	if (!soc)
@@ -827,8 +828,9 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 
 	list_for_each_entry(group, &smmu->groups, list)
 		if (group->soc == soc) {
+			grp = iommu_group_ref_get(group->group);
 			mutex_unlock(&smmu->lock);
-			return group->group;
+			return grp;
 		}
 
 	group = devm_kzalloc(smmu->dev, sizeof(*group), GFP_KERNEL);
-- 
2.27.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] iommu/tegra-smmu: Prune IOMMU group when it is released
  2020-08-06 15:54 ` Thierry Reding
@ 2020-08-06 15:54   ` Thierry Reding
  -1 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Jon Hunter, Krishna Reddy, iommu, linux-tegra

From: Thierry Reding <treding@nvidia.com>

In order to share groups between multiple devices we keep track of them
in a per-SMMU list. When an IOMMU group is released, a dangling pointer
to it stays around in that list. Fix this by implementing an IOMMU data
release callback for groups where the dangling pointer can be removed.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index c439c0929ef8..2574e716086b 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -19,6 +19,7 @@
 
 struct tegra_smmu_group {
 	struct list_head list;
+	struct tegra_smmu *smmu;
 	const struct tegra_smmu_group_soc *soc;
 	struct iommu_group *group;
 };
@@ -813,6 +814,16 @@ tegra_smmu_find_group(struct tegra_smmu *smmu, unsigned int swgroup)
 	return NULL;
 }
 
+static void tegra_smmu_group_release(void *iommu_data)
+{
+	struct tegra_smmu_group *group = iommu_data;
+	struct tegra_smmu *smmu = group->smmu;
+
+	mutex_lock(&smmu->lock);
+	list_del(&group->list);
+	mutex_unlock(&smmu->lock);
+}
+
 static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 						unsigned int swgroup)
 {
@@ -840,6 +851,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 	}
 
 	INIT_LIST_HEAD(&group->list);
+	group->smmu = smmu;
 	group->soc = soc;
 
 	group->group = iommu_group_alloc();
@@ -849,6 +861,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 		return NULL;
 	}
 
+	iommu_group_set_iommudata(group->group, group, tegra_smmu_group_release);
 	iommu_group_set_name(group->group, soc->name);
 	list_add_tail(&group->list, &smmu->groups);
 	mutex_unlock(&smmu->lock);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] iommu/tegra-smmu: Prune IOMMU group when it is released
@ 2020-08-06 15:54   ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2020-08-06 15:54 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-tegra, iommu, Jon Hunter

From: Thierry Reding <treding@nvidia.com>

In order to share groups between multiple devices we keep track of them
in a per-SMMU list. When an IOMMU group is released, a dangling pointer
to it stays around in that list. Fix this by implementing an IOMMU data
release callback for groups where the dangling pointer can be removed.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index c439c0929ef8..2574e716086b 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -19,6 +19,7 @@
 
 struct tegra_smmu_group {
 	struct list_head list;
+	struct tegra_smmu *smmu;
 	const struct tegra_smmu_group_soc *soc;
 	struct iommu_group *group;
 };
@@ -813,6 +814,16 @@ tegra_smmu_find_group(struct tegra_smmu *smmu, unsigned int swgroup)
 	return NULL;
 }
 
+static void tegra_smmu_group_release(void *iommu_data)
+{
+	struct tegra_smmu_group *group = iommu_data;
+	struct tegra_smmu *smmu = group->smmu;
+
+	mutex_lock(&smmu->lock);
+	list_del(&group->list);
+	mutex_unlock(&smmu->lock);
+}
+
 static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 						unsigned int swgroup)
 {
@@ -840,6 +851,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 	}
 
 	INIT_LIST_HEAD(&group->list);
+	group->smmu = smmu;
 	group->soc = soc;
 
 	group->group = iommu_group_alloc();
@@ -849,6 +861,7 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu,
 		return NULL;
 	}
 
+	iommu_group_set_iommudata(group->group, group, tegra_smmu_group_release);
 	iommu_group_set_name(group->group, soc->name);
 	list_add_tail(&group->list, &smmu->groups);
 	mutex_unlock(&smmu->lock);
-- 
2.27.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] iommu/tegra-smmu: Reference count fixes
  2020-08-06 15:54 ` Thierry Reding
@ 2020-09-04  9:00   ` Joerg Roedel
  -1 siblings, 0 replies; 10+ messages in thread
From: Joerg Roedel @ 2020-09-04  9:00 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Jon Hunter, Krishna Reddy, iommu, linux-tegra

On Thu, Aug 06, 2020 at 05:54:01PM +0200, Thierry Reding wrote:
> Thierry Reding (3):
>   iommu/tegra-smmu: Set IOMMU group name
>   iommu/tegra-smmu: Balance IOMMU group reference count
>   iommu/tegra-smmu: Prune IOMMU group when it is released

Applied, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] iommu/tegra-smmu: Reference count fixes
@ 2020-09-04  9:00   ` Joerg Roedel
  0 siblings, 0 replies; 10+ messages in thread
From: Joerg Roedel @ 2020-09-04  9:00 UTC (permalink / raw)
  To: Thierry Reding; +Cc: linux-tegra, iommu, Jon Hunter

On Thu, Aug 06, 2020 at 05:54:01PM +0200, Thierry Reding wrote:
> Thierry Reding (3):
>   iommu/tegra-smmu: Set IOMMU group name
>   iommu/tegra-smmu: Balance IOMMU group reference count
>   iommu/tegra-smmu: Prune IOMMU group when it is released

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-09-04  9:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06 15:54 [PATCH 0/3] iommu/tegra-smmu: Reference count fixes Thierry Reding
2020-08-06 15:54 ` Thierry Reding
2020-08-06 15:54 ` [PATCH 1/3] iommu/tegra-smmu: Set IOMMU group name Thierry Reding
2020-08-06 15:54   ` Thierry Reding
2020-08-06 15:54 ` [PATCH 2/3] iommu/tegra-smmu: Balance IOMMU group reference count Thierry Reding
2020-08-06 15:54   ` Thierry Reding
2020-08-06 15:54 ` [PATCH 3/3] iommu/tegra-smmu: Prune IOMMU group when it is released Thierry Reding
2020-08-06 15:54   ` Thierry Reding
2020-09-04  9:00 ` [PATCH 0/3] iommu/tegra-smmu: Reference count fixes Joerg Roedel
2020-09-04  9:00   ` Joerg Roedel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.