[PATCH 0/2] More IOMMU Fixes

[PATCH 0/2] More IOMMU Fixes

[Joerg Roedel]

Hi,

here are two patches that fix valid issues discovered by
Coverity. Please review.

Thanks,

	Joerg

Joerg Roedel (2):
  iommu: Make iommu_group_get_for_dev() more robust
  iommu/vt-d: Check return value of acpi_bus_get_device()

 drivers/iommu/dmar.c  | 3 +--
 drivers/iommu/iommu.c | 8 +++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH 1/2] iommu: Make iommu_group_get_for_dev() more robust

[Joerg Roedel]

From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

When a non-PCI device is passed to that function it might
pass group == NULL to iommu_group_add_device() which then
dereferences it and cause a crash this way. Fix it by
just returning an error for non-PCI devices.

Fixes: 104a1c13ac66e40cf8c6ae74d76ff14ff24b9b01
Cc: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/iommu.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1698360..ef8da12 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -678,15 +678,17 @@ static struct iommu_group *iommu_group_get_for_pci_dev(struct pci_dev *pdev)
  */
 struct iommu_group *iommu_group_get_for_dev(struct device *dev)
 {
-	struct iommu_group *group = ERR_PTR(-EIO);
+	struct iommu_group *group;
 	int ret;
 
 	group = iommu_group_get(dev);
 	if (group)
 		return group;
 
-	if (dev_is_pci(dev))
-		group = iommu_group_get_for_pci_dev(to_pci_dev(dev));
+	if (!dev_is_pci(dev))
+		return ERR_PTR(-EINVAL);
+
+	group = iommu_group_get_for_pci_dev(to_pci_dev(dev));
 
 	if (IS_ERR(group))
 		return group;
-- 
1.9.1

Re: [PATCH 1/2] iommu: Make iommu_group_get_for_dev() more robust

[Alex Williamson]

On Thu, 2014-08-21 at 23:37 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> 
> When a non-PCI device is passed to that function it might
> pass group == NULL to iommu_group_add_device() which then
> dereferences it and cause a crash this way. Fix it by
> just returning an error for non-PCI devices.
> 
> Fixes: 104a1c13ac66e40cf8c6ae74d76ff14ff24b9b01
> Cc: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> ---
>  drivers/iommu/iommu.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 1698360..ef8da12 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -678,15 +678,17 @@ static struct iommu_group *iommu_group_get_for_pci_dev(struct pci_dev *pdev)
>   */
>  struct iommu_group *iommu_group_get_for_dev(struct device *dev)
>  {
> -	struct iommu_group *group = ERR_PTR(-EIO);
> +	struct iommu_group *group;
>  	int ret;
>  
>  	group = iommu_group_get(dev);
>  	if (group)
>  		return group;

Hmm, I bet I had a second pointer for this case but mistakenly optimized
it out refining the patch.  This solution works too though.

Acked-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Thanks!
Alex

>  
> -	if (dev_is_pci(dev))
> -		group = iommu_group_get_for_pci_dev(to_pci_dev(dev));
> +	if (!dev_is_pci(dev))
> +		return ERR_PTR(-EINVAL);
> +
> +	group = iommu_group_get_for_pci_dev(to_pci_dev(dev));
>  
>  	if (IS_ERR(group))
>  		return group;

[PATCH 2/2] iommu/vt-d: Check return value of acpi_bus_get_device()

[Joerg Roedel]

From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Checking adev == NULL is not sufficient as
acpi_bus_get_device() might not touch the value of this
parameter in an error case, so check the return value
directly.

Fixes: ed40356b5fcf1ce28e026ab39c5b2b6939068b50
Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/dmar.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 60ab474..06d268a 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -678,8 +678,7 @@ static int __init dmar_acpi_dev_scope_init(void)
 				       andd->device_name);
 				continue;
 			}
-			acpi_bus_get_device(h, &adev);
-			if (!adev) {
+			if (acpi_bus_get_device(h, &adev)) {
 				pr_err("Failed to get device for ACPI object %s\n",
 				       andd->device_name);
 				continue;
-- 
1.9.1

Re: [PATCH 2/2] iommu/vt-d: Check return value of acpi_bus_get_device()

[Alex Williamson]

On Thu, 2014-08-21 at 23:37 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> 
> Checking adev == NULL is not sufficient as
> acpi_bus_get_device() might not touch the value of this
> parameter in an error case, so check the return value
> directly.
> 
> Fixes: ed40356b5fcf1ce28e026ab39c5b2b6939068b50
> Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> ---
>  drivers/iommu/dmar.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index 60ab474..06d268a 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -678,8 +678,7 @@ static int __init dmar_acpi_dev_scope_init(void)
>  				       andd->device_name);
>  				continue;
>  			}
> -			acpi_bus_get_device(h, &adev);
> -			if (!adev) {
> +			if (acpi_bus_get_device(h, &adev)) {
>  				pr_err("Failed to get device for ACPI object %s\n",
>  				       andd->device_name);
>  				continue;

Reviewed-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>