Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1706874 > unrolled thread

[PATCH] vfio: fix noiommu vfio_iommu_group_get reference count

Back to article view | Back to linux.kernel

Contents

  [PATCH] vfio: fix noiommu vfio_iommu_group_get reference count Eric Auger <eric.auger@redhat.com> - 2017-08-08 22:50 +0200
    Re: [PATCH] vfio: fix noiommu vfio_iommu_group_get reference count Alex Williamson <alex.williamson@redhat.com> - 2017-08-10 21:50 +0200

#1706874 — [PATCH] vfio: fix noiommu vfio_iommu_group_get reference count

In vfio_iommu_group_get() we want to increase the reference
count of the iommu group.

In noiommu case, the group does not exist and is allocated.
iommu_group_add_device() increases the group ref count. However we
then call iommu_group_put() which decrements it.

This leads to a "refcount_t: underflow WARN_ON".

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/vfio/vfio.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 330d505..fd8d691 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -138,7 +138,6 @@ struct iommu_group *vfio_iommu_group_get(struct device *dev)
 	iommu_group_set_name(group, "vfio-noiommu");
 	iommu_group_set_iommudata(group, &noiommu, NULL);
 	ret = iommu_group_add_device(group, dev);
-	iommu_group_put(group);
 	if (ret)
 		return NULL;
 
-- 
2.5.5

[toc] | [next] | [standalone]

#1708988

On Tue,  8 Aug 2017 22:44:28 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> In vfio_iommu_group_get() we want to increase the reference
> count of the iommu group.
> 
> In noiommu case, the group does not exist and is allocated.
> iommu_group_add_device() increases the group ref count. However we
> then call iommu_group_put() which decrements it.
> 
> This leads to a "refcount_t: underflow WARN_ON".

Yep, the group is created with an initial reference count of 1, we then
add the device, which increments the reference count.  Normally the
instantiator of the group would then release the reference, so that
only the device reference holds the group.  However here we want a
reference in addition to the device reference, so we should never have
released the initial reference.  Seems right, except...

> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  drivers/vfio/vfio.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 330d505..fd8d691 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -138,7 +138,6 @@ struct iommu_group *vfio_iommu_group_get(struct device *dev)
>  	iommu_group_set_name(group, "vfio-noiommu");
>  	iommu_group_set_iommudata(group, &noiommu, NULL);
>  	ret = iommu_group_add_device(group, dev);
> -	iommu_group_put(group);
>  	if (ret)
>  		return NULL;

We leak the group in the error case here.  Perhaps the 'put' is
correct, it was just typo'd outside of the error case.  Thanks,

Alex

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web