[PATCH] iommu: Check the result of iommu_group_get() for NULL

[PATCH] iommu: Check the result of iommu_group_get() for NULL

[Jordan Crouse]

The result of iommu_group_get() was being blindly used in both
attach and detach which results in a dereference when trying
to work with an unknown device.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/iommu/iommu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3de5c0b..69fef99 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1303,6 +1303,9 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 	int ret;
 
 	group = iommu_group_get(dev);
+	if (!group)
+		return -ENODEV;
+
 	/*
 	 * Lock the group to make sure the device-count doesn't
 	 * change while we are attaching
@@ -1341,6 +1344,8 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 	struct iommu_group *group;
 
 	group = iommu_group_get(dev);
+	if (!group)
+		return;
 
 	mutex_lock(&group->mutex);
 	if (iommu_group_device_count(group) != 1) {
-- 
1.9.1

Re: [PATCH] iommu: Check the result of iommu_group_get() for NULL

[Alex Williamson]

On Mon,  4 Dec 2017 10:22:10 -0700
Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:

> The result of iommu_group_get() was being blindly used in both
> attach and detach which results in a dereference when trying
> to work with an unknown device.
> 
> Signed-off-by: Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  drivers/iommu/iommu.c | 5 +++++
>  1 file changed, 5 insertions(+)

Applied to v4.16-iommu/misc.  Thanks,

Alex

Re: [PATCH] iommu: Check the result of iommu_group_get() for NULL

[Robin Murphy]

Hi Jordan,

On 04/12/17 17:22, Jordan Crouse wrote:
> The result of iommu_group_get() was being blindly used in both
> attach and detach which results in a dereference when trying
> to work with an unknown device.

I missed this one before it got applied, but FWIW that behaviour was 
actually somewhat intentional (see 05f80300dc8b). Callers really 
shouldn't be trying to attach random unknown devices to IOMMU domains 
anyway, but more crucially, now that iommu_{attach,detach}_group() work 
everywhere I think the long-term plan is still to remove 
iommu_{attach,detach}_device() completely.

I have no great objection to this patch myself, but do consider it a 
heads-up that any callers it fixes are liable to be ridden roughshod 
over by further API changes in future.

Robin.

> Signed-off-by: Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>   drivers/iommu/iommu.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 3de5c0b..69fef99 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1303,6 +1303,9 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
>   	int ret;
>   
>   	group = iommu_group_get(dev);
> +	if (!group)
> +		return -ENODEV;
> +
>   	/*
>   	 * Lock the group to make sure the device-count doesn't
>   	 * change while we are attaching
> @@ -1341,6 +1344,8 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
>   	struct iommu_group *group;
>   
>   	group = iommu_group_get(dev);
> +	if (!group)
> +		return;
>   
>   	mutex_lock(&group->mutex);
>   	if (iommu_group_device_count(group) != 1) {
>