Re: [PATCH] iommufd/selftest:fix a resource leak

[Jason Gunthorpe]

On Wed, Sep 04, 2024 at 04:08:06AM +0800, Liu Jing wrote:
> If the file fails to open, not only return return1 but also close the file 
> descriptor,otherwise resource
> leak will occur

Why doesn't FIXURE_TEARDOWN do this?

Jason

[PATCH] iommufd/selftest:fix a resource leak

[Liu Jing]

If the file fails to open, not only return return1 but also close the file 
descriptor,otherwise resource
leak will occur

Signed-off-by: Liu Jing 
---
 .../selftests/iommu/iommufd_fail_nth.c| 143 ++
 1 file changed, 83 insertions(+), 60 deletions(-)

diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c 
b/tools/testing/selftests/iommu/iommufd_fail_nth.c
index c5d5e69452b0..ff4e5d8aad57 100644
--- a/tools/testing/selftests/iommu/iommufd_fail_nth.c
+++ b/tools/testing/selftests/iommu/iommufd_fail_nth.c
@@ -237,10 +237,10 @@ TEST_FAIL_NTH(basic_fail_nth, basic)
 
self->fd = open("/dev/iommu", O_RDWR);
if (self->fd == -1)
-   return -1;
+   goto close;
 
if (_test_ioctl_ioas_alloc(self->fd, &ioas_id))
-   return -1;
+   goto close;
 
{
struct iommu_ioas_iova_ranges ranges_cmd = {
@@ -250,7 +250,7 @@ TEST_FAIL_NTH(basic_fail_nth, basic)
.allowed_iovas = (uintptr_t)ranges,
};
if (ioctl(self->fd, IOMMU_IOAS_IOVA_RANGES, &ranges_cmd))
-   return -1;
+   goto close;
}
 
{
@@ -264,13 +264,13 @@ TEST_FAIL_NTH(basic_fail_nth, basic)
ranges[0].start = 16*1024;
ranges[0].last = BUFFER_SIZE + 16 * 1024 * 600 - 1;
if (ioctl(self->fd, IOMMU_IOAS_ALLOW_IOVAS, &allow_cmd))
-   return -1;
+   goto close;
}
 
if (_test_ioctl_ioas_map(self->fd, ioas_id, buffer, BUFFER_SIZE, &iova,
 IOMMU_IOAS_MAP_WRITEABLE |
 IOMMU_IOAS_MAP_READABLE))
-   return -1;
+   goto close;
 
{
struct iommu_ioas_copy copy_cmd = {
@@ -284,15 +284,19 @@ TEST_FAIL_NTH(basic_fail_nth, basic)
};
 
if (ioctl(self->fd, IOMMU_IOAS_COPY, ©_cmd))
-   return -1;
+   goto close;
}
 
if (_test_ioctl_ioas_unmap(self->fd, ioas_id, iova, BUFFER_SIZE,
   NULL))
-   return -1;
+   goto close;
/* Failure path of no IOVA to unmap */
_test_ioctl_ioas_unmap(self->fd, ioas_id, iova, BUFFER_SIZE, NULL);
return 0;
+
+close:
+   close(self->fd);
+   return -1;
 }
 
 /* iopt_area_fill_domains() and iopt_area_fill_domain() */
@@ -305,30 +309,33 @@ TEST_FAIL_NTH(basic_fail_nth, map_domain)
 
self->fd = open("/dev/iommu", O_RDWR);
if (self->fd == -1)
-   return -1;
+   goto close;
 
if (_test_ioctl_ioas_alloc(self->fd, &ioas_id))
-   return -1;
+   goto close;
 
if (_test_ioctl_set_temp_memory_limit(self->fd, 32))
-   return -1;
+   goto close;
 
fail_nth_enable();
 
if (_test_cmd_mock_domain(self->fd, ioas_id, &stdev_id, &hwpt_id, NULL))
-   return -1;
+   goto close;
 
if (_test_ioctl_ioas_map(self->fd, ioas_id, buffer, 262144, &iova,
 IOMMU_IOAS_MAP_WRITEABLE |
 IOMMU_IOAS_MAP_READABLE))
-   return -1;
+   goto close;
 
if (_test_ioctl_destroy(self->fd, stdev_id))
-   return -1;
+   goto close;
 
if (_test_cmd_mock_domain(self->fd, ioas_id, &stdev_id, &hwpt_id, NULL))
-   return -1;
+   goto close;
return 0;
+close:
+   close(self->fd);
+   return -1;
 }
 
 TEST_FAIL_NTH(basic_fail_nth, map_two_domains)
@@ -342,40 +349,43 @@ TEST_FAIL_NTH(basic_fail_nth, map_two_domains)
 
self->fd = open("/dev/iommu", O_RDWR);
if (self->fd == -1)
-   return -1;
+   goto close;
 
if (_test_ioctl_ioas_alloc(self->fd, &ioas_id))
-   return -1;
+   goto close;
 
if (_test_ioctl_set_temp_memory_limit(self->fd, 32))
-   return -1;
+   goto close;
 
if (_test_cmd_mock_domain(self->fd, ioas_id, &stdev_id, &hwpt_id, NULL))
-   return -1;
+   goto close;
 
fail_nth_enable();
 
if (_test_cmd_mock_domain(self->fd, ioas_id, &stdev_id2, &hwpt_id2,
  NULL))
-   return -1;
+   goto close;
 
if (_test_ioctl_ioas_map(self->fd, ioas_id, buffer, 262144, &iova,
 IOMMU_IOAS_MAP_WRITEABLE |
 IOMMU_IOAS_MAP_READABLE))
-   return -1;
+   goto close;
 
if (_test_ioctl_destroy(self->fd, stdev_id))
-   return -1;
+   goto close;
 
if (_test_ioctl_destroy(self->fd, stdev_id2))
-   return -1;
+   goto clo