There are a few patches in vIRQ series that rework the fault.c file. So, we should fix this before that bigger series touches the same code.
And add missing coverage for IOMMU_FAULT_QUEUE_ALLOC in iommufd_fail_nth.
Thanks! Nicolin
Nicolin Chen (2): iommufd/fault: Fix out_fput in iommufd_fault_alloc() iommufd/selftest: Cover IOMMU_FAULT_QUEUE_ALLOC in iommufd_fail_nth
drivers/iommu/iommufd/fault.c | 2 -- tools/testing/selftests/iommu/iommufd_fail_nth.c | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)
As fput() calls the file->f_op->release op, where fault obj and ictx are getting released, there is no need to release these two after fput() one more time, which would result in imbalanced refcounts: refcount_t: decrement hit 0; leaking memory. WARNING: CPU: 48 PID: 2369 at lib/refcount.c:31 refcount_warn_saturate+0x60/0x230 Call trace: refcount_warn_saturate+0x60/0x230 (P) refcount_warn_saturate+0x60/0x230 (L) iommufd_fault_fops_release+0x9c/0xe0 [iommufd] ... VFS: Close: file count is 0 (f_op=iommufd_fops [iommufd]) WARNING: CPU: 48 PID: 2369 at fs/open.c:1507 filp_flush+0x3c/0xf0 Call trace: filp_flush+0x3c/0xf0 (P) filp_flush+0x3c/0xf0 (L) __arm64_sys_close+0x34/0x98 ... imbalanced put on file reference count WARNING: CPU: 48 PID: 2369 at fs/file.c:74 __file_ref_put+0x100/0x138 Call trace: __file_ref_put+0x100/0x138 (P) __file_ref_put+0x100/0x138 (L) __fput_sync+0x4c/0xd0
Drop those two lines to fix the warnings above.
Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object") Cc: stable@vger.kernel.org Signed-off-by: Nicolin Chen nicolinc@nvidia.com --- drivers/iommu/iommufd/fault.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c index 053b0e30f55a..1fe804e28a86 100644 --- a/drivers/iommu/iommufd/fault.c +++ b/drivers/iommu/iommufd/fault.c @@ -420,8 +420,6 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) put_unused_fd(fdno); out_fput: fput(filep); - refcount_dec(&fault->obj.users); - iommufd_ctx_put(fault->ictx); out_abort: iommufd_object_abort_and_destroy(ucmd->ictx, &fault->obj);
On 2024/12/3 16:02, Nicolin Chen wrote:
As fput() calls the file->f_op->release op, where fault obj and ictx are getting released, there is no need to release these two after fput() one more time, which would result in imbalanced refcounts: refcount_t: decrement hit 0; leaking memory. WARNING: CPU: 48 PID: 2369 at lib/refcount.c:31 refcount_warn_saturate+0x60/0x230 Call trace: refcount_warn_saturate+0x60/0x230 (P) refcount_warn_saturate+0x60/0x230 (L) iommufd_fault_fops_release+0x9c/0xe0 [iommufd] ... VFS: Close: file count is 0 (f_op=iommufd_fops [iommufd]) WARNING: CPU: 48 PID: 2369 at fs/open.c:1507 filp_flush+0x3c/0xf0 Call trace: filp_flush+0x3c/0xf0 (P) filp_flush+0x3c/0xf0 (L) __arm64_sys_close+0x34/0x98 ... imbalanced put on file reference count WARNING: CPU: 48 PID: 2369 at fs/file.c:74 __file_ref_put+0x100/0x138 Call trace: __file_ref_put+0x100/0x138 (P) __file_ref_put+0x100/0x138 (L) __fput_sync+0x4c/0xd0
Drop those two lines to fix the warnings above.
Fixes: 07838f7fd529 ("iommufd: Add iommufd fault object") Cc: stable@vger.kernel.org Signed-off-by: Nicolin Chen nicolinc@nvidia.com
drivers/iommu/iommufd/fault.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c index 053b0e30f55a..1fe804e28a86 100644 --- a/drivers/iommu/iommufd/fault.c +++ b/drivers/iommu/iommufd/fault.c @@ -420,8 +420,6 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) put_unused_fd(fdno); out_fput: fput(filep);
- refcount_dec(&fault->obj.users);
- iommufd_ctx_put(fault->ictx); out_abort: iommufd_object_abort_and_destroy(ucmd->ictx, &fault->obj);
Reviewed-by: Yi Liu yi.l.liu@intel.com
This was missing in the series introducing the fault object. Thus, add it.
Signed-off-by: Nicolin Chen nicolinc@nvidia.com --- tools/testing/selftests/iommu/iommufd_fail_nth.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c b/tools/testing/selftests/iommu/iommufd_fail_nth.c index 22f6fd5f0f74..64b1f8e1b0cf 100644 --- a/tools/testing/selftests/iommu/iommufd_fail_nth.c +++ b/tools/testing/selftests/iommu/iommufd_fail_nth.c @@ -615,7 +615,12 @@ TEST_FAIL_NTH(basic_fail_nth, access_pin_domain) /* device.c */ TEST_FAIL_NTH(basic_fail_nth, device) { + struct iommu_hwpt_selftest data = { + .iotlb = IOMMU_TEST_IOTLB_DEFAULT, + }; struct iommu_test_hw_info info; + uint32_t fault_id, fault_fd; + uint32_t fault_hwpt_id; uint32_t ioas_id; uint32_t ioas_id2; uint32_t stdev_id; @@ -678,6 +683,15 @@ TEST_FAIL_NTH(basic_fail_nth, device) if (_test_cmd_vdevice_alloc(self->fd, viommu_id, idev_id, 0, &vdev_id)) return -1;
+ if (_test_ioctl_fault_alloc(self->fd, &fault_id, &fault_fd)) + return -1; + close(fault_fd); + + if (_test_cmd_hwpt_alloc(self->fd, idev_id, hwpt_id, fault_id, + IOMMU_HWPT_FAULT_ID_VALID, &fault_hwpt_id, + IOMMU_HWPT_DATA_SELFTEST, &data, sizeof(data))) + return -1; + return 0; }
On Tue, Dec 03, 2024 at 12:02:53AM -0800, Nicolin Chen wrote:
There are a few patches in vIRQ series that rework the fault.c file. So, we should fix this before that bigger series touches the same code.
And add missing coverage for IOMMU_FAULT_QUEUE_ALLOC in iommufd_fail_nth.
Thanks! Nicolin
Nicolin Chen (2): iommufd/fault: Fix out_fput in iommufd_fault_alloc() iommufd/selftest: Cover IOMMU_FAULT_QUEUE_ALLOC in iommufd_fail_nth
Applied to for-rc, thanks
Jason
linux-kselftest-mirror@lists.linaro.org