Skip to content

Commit 8f33bb2

Browse files
bostroessermartinkpetersen
authored andcommitted
scsi: target: tcmu: Fix memory leak caused by wrong uio usage
When user deletes a tcmu device via configFS, tcmu calls uio_unregister_device(). During that call uio resets its pointer to struct uio_info provided by tcmu. That means, after uio_unregister_device() uio will no longer execute any of the callbacks tcmu had set in uio_info. Especially, if userspace daemon still holds the corresponding uio device open or mmap'ed while tcmu calls uio_unregister_device(), uio will not call tcmu_release() when userspace finally closes and munmaps the uio device. Since tcmu does refcounting for the tcmu device in tcmu_open() and tcmu_release(), in the decribed case refcount does not drop to 0 and tcmu does not free tcmu device's resources. In extreme cases this can cause memory leaking of up to 1 GB for a single tcmu device. After uio_unregister_device(), uio will reject every open, read, write, mmap from userspace with -EOI. But userspace daemon can still access the mmap'ed command ring and data area. Therefore tcmu should wait until userspace munmaps the uio device before it frees the resources, as we don't want to cause SIGSEGV or SIGBUS to user space. That said, current refcounting during tcmu_open and tcmu_release does not work correctly, and refcounting better should be done in the open and close callouts of the vm_operations_struct, which tcmu assigns to each mmap of the uio device (because it wants its own page fault handler). This patch fixes the memory leak by removing refcounting from tcmu_open and tcmu_close, and instead adding new tcmu_vma_open() and tcmu_vma_close() handlers that only do refcounting. Link: https://lore.kernel.org/r/20210218175039.7829-3-bostroesser@gmail.com Reviewed-by: Mike Christie <michael.christie@oracle.com> Signed-off-by: Bodo Stroesser <bostroesser@gmail.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 43bf922 commit 8f33bb2

1 file changed

Lines changed: 26 additions & 3 deletions

File tree

drivers/target/target_core_user.c

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,6 +1643,8 @@ static void tcmu_dev_kref_release(struct kref *kref)
16431643
bitmap_free(udev->data_bitmap);
16441644
mutex_unlock(&udev->cmdr_lock);
16451645

1646+
pr_debug("dev_kref_release\n");
1647+
16461648
call_rcu(&dev->rcu_head, tcmu_dev_call_rcu);
16471649
}
16481650

@@ -1758,6 +1760,25 @@ static struct page *tcmu_try_get_block_page(struct tcmu_dev *udev, uint32_t dbi)
17581760
return page;
17591761
}
17601762

1763+
static void tcmu_vma_open(struct vm_area_struct *vma)
1764+
{
1765+
struct tcmu_dev *udev = vma->vm_private_data;
1766+
1767+
pr_debug("vma_open\n");
1768+
1769+
kref_get(&udev->kref);
1770+
}
1771+
1772+
static void tcmu_vma_close(struct vm_area_struct *vma)
1773+
{
1774+
struct tcmu_dev *udev = vma->vm_private_data;
1775+
1776+
pr_debug("vma_close\n");
1777+
1778+
/* release ref from tcmu_vma_open */
1779+
kref_put(&udev->kref, tcmu_dev_kref_release);
1780+
}
1781+
17611782
static vm_fault_t tcmu_vma_fault(struct vm_fault *vmf)
17621783
{
17631784
struct tcmu_dev *udev = vmf->vma->vm_private_data;
@@ -1796,6 +1817,8 @@ static vm_fault_t tcmu_vma_fault(struct vm_fault *vmf)
17961817
}
17971818

17981819
static const struct vm_operations_struct tcmu_vm_ops = {
1820+
.open = tcmu_vma_open,
1821+
.close = tcmu_vma_close,
17991822
.fault = tcmu_vma_fault,
18001823
};
18011824

@@ -1812,6 +1835,8 @@ static int tcmu_mmap(struct uio_info *info, struct vm_area_struct *vma)
18121835
if (vma_pages(vma) != (udev->ring_size >> PAGE_SHIFT))
18131836
return -EINVAL;
18141837

1838+
tcmu_vma_open(vma);
1839+
18151840
return 0;
18161841
}
18171842

@@ -1824,7 +1849,6 @@ static int tcmu_open(struct uio_info *info, struct inode *inode)
18241849
return -EBUSY;
18251850

18261851
udev->inode = inode;
1827-
kref_get(&udev->kref);
18281852

18291853
pr_debug("open\n");
18301854

@@ -1838,8 +1862,7 @@ static int tcmu_release(struct uio_info *info, struct inode *inode)
18381862
clear_bit(TCMU_DEV_BIT_OPEN, &udev->flags);
18391863

18401864
pr_debug("close\n");
1841-
/* release ref from open */
1842-
kref_put(&udev->kref, tcmu_dev_kref_release);
1865+
18431866
return 0;
18441867
}
18451868

0 commit comments

Comments
 (0)