* [PATCH] iommu: Don't take group reference in iommu_alloc_default_domain()
@ 2020-05-25 13:01 Joerg Roedel
2020-05-25 13:24 ` Sai Prakash Ranjan
0 siblings, 1 reply; 2+ messages in thread
From: Joerg Roedel @ 2020-05-25 13:01 UTC (permalink / raw)
To: Joerg Roedel; +Cc: iommu, linux-kernel, Joerg Roedel, Sai Prakash Ranjan
From: Joerg Roedel <jroedel@suse.de>
The iommu_alloc_default_domain() function takes a reference to an IOMMU
group without releasing it. This causes the group to never be released,
with undefined side effects.
The function has only one call-site, which takes a group reference on
its own, so to fix this leak, do not take another reference in
iommu_alloc_default_domain() and pass the group as a function parameter
instead.
Reference: https://lore.kernel.org/lkml/20200522130145.30067-1-saiprakash.ranjan@codeaurora.org/
Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Fixes: 6e1aa2049154 ("iommu: Move default domain allocation to iommu_probe_device()")
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/iommu/iommu.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 374b34fd6fac..bf20674769e0 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -80,7 +80,8 @@ static bool iommu_cmd_line_dma_api(void)
return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API);
}
-static int iommu_alloc_default_domain(struct device *dev);
+static int iommu_alloc_default_domain(struct iommu_group *group,
+ struct device *dev);
static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
unsigned type);
static int __iommu_attach_device(struct iommu_domain *domain,
@@ -251,17 +252,17 @@ int iommu_probe_device(struct device *dev)
if (ret)
goto err_out;
+ group = iommu_group_get(dev);
+ if (!group)
+ goto err_release;
+
/*
* Try to allocate a default domain - needs support from the
* IOMMU driver. There are still some drivers which don't
* support default domains, so the return value is not yet
* checked.
*/
- iommu_alloc_default_domain(dev);
-
- group = iommu_group_get(dev);
- if (!group)
- goto err_release;
+ iommu_alloc_default_domain(group, dev);
if (group->default_domain)
ret = __iommu_attach_device(group->default_domain, dev);
@@ -1478,15 +1479,11 @@ static int iommu_group_alloc_default_domain(struct bus_type *bus,
return 0;
}
-static int iommu_alloc_default_domain(struct device *dev)
+static int iommu_alloc_default_domain(struct iommu_group *group,
+ struct device *dev)
{
- struct iommu_group *group;
unsigned int type;
- group = iommu_group_get(dev);
- if (!group)
- return -ENODEV;
-
if (group->default_domain)
return 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] iommu: Don't take group reference in iommu_alloc_default_domain()
2020-05-25 13:01 [PATCH] iommu: Don't take group reference in iommu_alloc_default_domain() Joerg Roedel
@ 2020-05-25 13:24 ` Sai Prakash Ranjan
0 siblings, 0 replies; 2+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-25 13:24 UTC (permalink / raw)
To: Joerg Roedel; +Cc: iommu, linux-kernel, Joerg Roedel, saiprakash.ranjan
Hi Joerg,
On 2020-05-25 18:31, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> The iommu_alloc_default_domain() function takes a reference to an IOMMU
> group without releasing it. This causes the group to never be released,
> with undefined side effects.
>
> The function has only one call-site, which takes a group reference on
> its own, so to fix this leak, do not take another reference in
> iommu_alloc_default_domain() and pass the group as a function parameter
> instead.
>
> Reference:
> https://lore.kernel.org/lkml/20200522130145.30067-1-saiprakash.ranjan@codeaurora.org/
> Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Cc: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Fixes: 6e1aa2049154 ("iommu: Move default domain allocation to
> iommu_probe_device()")
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> drivers/iommu/iommu.c | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 374b34fd6fac..bf20674769e0 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -80,7 +80,8 @@ static bool iommu_cmd_line_dma_api(void)
> return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API);
> }
>
> -static int iommu_alloc_default_domain(struct device *dev);
> +static int iommu_alloc_default_domain(struct iommu_group *group,
> + struct device *dev);
> static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
> unsigned type);
> static int __iommu_attach_device(struct iommu_domain *domain,
> @@ -251,17 +252,17 @@ int iommu_probe_device(struct device *dev)
> if (ret)
> goto err_out;
>
> + group = iommu_group_get(dev);
> + if (!group)
> + goto err_release;
> +
> /*
> * Try to allocate a default domain - needs support from the
> * IOMMU driver. There are still some drivers which don't
> * support default domains, so the return value is not yet
> * checked.
> */
> - iommu_alloc_default_domain(dev);
> -
> - group = iommu_group_get(dev);
> - if (!group)
> - goto err_release;
> + iommu_alloc_default_domain(group, dev);
>
> if (group->default_domain)
> ret = __iommu_attach_device(group->default_domain, dev);
> @@ -1478,15 +1479,11 @@ static int
> iommu_group_alloc_default_domain(struct bus_type *bus,
> return 0;
> }
>
> -static int iommu_alloc_default_domain(struct device *dev)
> +static int iommu_alloc_default_domain(struct iommu_group *group,
> + struct device *dev)
> {
> - struct iommu_group *group;
> unsigned int type;
>
> - group = iommu_group_get(dev);
> - if (!group)
> - return -ENODEV;
> -
> if (group->default_domain)
> return 0;
Tested this out and it works for me.
Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-05-25 13:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-25 13:01 [PATCH] iommu: Don't take group reference in iommu_alloc_default_domain() Joerg Roedel
2020-05-25 13:24 ` Sai Prakash Ranjan
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).