Skip to contentSkip to conversation

vfio: fix noiommu vfio_iommu_group_get reference count

PATCH Superseded Eric Auger 2 participants 2 messages Aug 8, 2017 – Aug 10, 2017 VFIO driver

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Eric Auger

author
#0 lore raw

PATCH 1/1 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".

---
 drivers/vfio/vfio.c | 1 -
 1 file changed, 1 deletion(-)
drivers/vfio/vfio.c +0 -1
index 330d505..fd8d691 100644
@@ -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;
 
Signature · 1 line
2.5.5
#1 lore raw
Quoted 8 lines · On Tue, 8 Aug 2017 22:44:28 +0200 Eric Auger <[email protected]> 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...

Quoted 10 lines
---
 drivers/vfio/vfio.c | 1 -
 1 file changed, 1 deletion(-)
drivers/vfio/vfio.c +0 -0
index 330d505..fd8d691 100644

drivers/vfio/vfio.c struct iommu_group *vfio_iommu_group_get(struct device *dev) · line 142

Quoted 6 lines
drivers/vfio/vfio.c +0 -1
@@ -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