diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 36dff7ca3ae4..f5836a5bdbbc 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -1144,7 +1144,7 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd) data = ops->hw_info(idev->dev, &data_len, &cmd->out_data_type); if (IS_ERR(data)) { rc = PTR_ERR(data); - goto err_put; + goto out_put; } /* @@ -1154,18 +1154,17 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd) if (WARN_ON_ONCE(cmd->out_data_type == IOMMU_HW_INFO_TYPE_NONE)) { rc = -ENODEV; - goto out; + goto out_free; } } else { cmd->out_data_type = IOMMU_HW_INFO_TYPE_NONE; data_len = 0; - data = NULL; } copy_len = min(cmd->data_len, data_len); if (copy_to_user(user_ptr, data, copy_len)) { rc = -EFAULT; - goto out; + goto out_free; } /* @@ -1175,7 +1174,7 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd) if (copy_len < cmd->data_len) { if (clear_user(user_ptr + copy_len, cmd->data_len - copy_len)) { rc = -EFAULT; - goto out; + goto out_free; } } @@ -1186,9 +1185,9 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd) cmd->data_len = data_len; rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd)); -out: +out_free: kfree(data); -err_put: +out_put: iommufd_put_object(&idev->obj); return rc; } diff --git a/include/linux/iommu.h b/include/linux/iommu.h index f2d6a3989713..bd6a1110b294 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -228,14 +228,10 @@ struct iommu_iotlb_gather { /** * struct iommu_ops - iommu ops and capabilities * @capable: check capability - * @hw_info: IOMMU hardware information. The type of the returned data is - * marked by the output type of this op. Type is one of - * enum iommu_hw_info_type defined in include/uapi/linux/iommufd.h. - * The drivers that support this op should define a unique type - * in include/uapi/linux/iommufd.h. The data buffer returned by this - * op is allocated in the IOMMU driver and the caller should free it - * after use. Return the data buffer if success, or ERR_PTR on - * failure. + * @hw_info: report iommu hardware information. The data buffer returned by this + * op is allocated in the iommu driver and freed by the caller after + * use. The information type is one of enum iommu_hw_info_type defined + * in include/uapi/linux/iommufd.h. * @domain_alloc: allocate iommu domain * @probe_device: Add device to iommu driver handling * @release_device: Remove device from iommu driver handling diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h index 42f024c07006..b4ba0c0cbab6 100644 --- a/include/uapi/linux/iommufd.h +++ b/include/uapi/linux/iommufd.h @@ -429,10 +429,7 @@ enum iommu_hw_info_type { * fills the buffer using the given length in @data_len. If the ioctl succeeds, * @data_len will be updated to the length that kernel actually supports, * @out_data_type will be filled to decode the data filled in the buffer - * pointed by @data_uptr. Input @data_len == zero is allowed, no information - * data will be filled to user, but user space could get the iommu_hw_info_type - * filled in @out_data_type and the iommu hardware information data length - * supported by kernel filled in @data_len. + * pointed by @data_uptr. Input @data_len == zero is allowed. */ struct iommu_hw_info { __u32 size; diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c index a068bfbe9f83..33d08600be13 100644 --- a/tools/testing/selftests/iommu/iommufd.c +++ b/tools/testing/selftests/iommu/iommufd.c @@ -295,10 +295,13 @@ TEST_F(iommufd_ioas, ioas_area_auto_destroy) TEST_F(iommufd_ioas, get_hw_info) { struct iommu_test_hw_info buffer_exact; - struct iommu_test_hw_info_buffer { + struct iommu_test_hw_info_buffer_larger { struct iommu_test_hw_info info; uint64_t trailing_bytes; } buffer_larger; + struct iommu_test_hw_info_buffer_smaller { + __u32 flags; + } buffer_smaller; if (self->device_id) { /* Provide a zero-size user_buffer */ @@ -310,6 +313,11 @@ TEST_F(iommufd_ioas, get_hw_info) * kernel zero the trailing bytes. */ test_cmd_get_hw_info(self->device_id, &buffer_larger, sizeof(buffer_larger)); + /* + * Provide a user_buffer with size smaller than the exact size to check if + * the fields within the size range still gets updated. + */ + test_cmd_get_hw_info(self->device_id, &buffer_smaller, sizeof(buffer_smaller)); } else { test_err_get_hw_info(ENOENT, self->device_id, &buffer_exact, sizeof(buffer_exact)); diff --git a/tools/testing/selftests/iommu/iommufd_utils.h b/tools/testing/selftests/iommu/iommufd_utils.h index 097418f2fdb3..e0753d03ecaa 100644 --- a/tools/testing/selftests/iommu/iommufd_utils.h +++ b/tools/testing/selftests/iommu/iommufd_utils.h @@ -21,6 +21,10 @@ static unsigned long BUFFER_SIZE; static unsigned long PAGE_SIZE; +#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) +#define offsetofend(TYPE, MEMBER) \ + (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER)) + /* * Have the kernel check the refcount on pages. I don't know why a freshly * mmap'd anon non-compound page starts out with a ref of 3 @@ -353,13 +357,13 @@ static void teardown_iommufd(int fd, struct __test_metadata *_metadata) static int _test_cmd_get_hw_info(int fd, __u32 device_id, void *data, size_t data_len) { + struct iommu_test_hw_info *info = (struct iommu_test_hw_info *)data; struct iommu_hw_info cmd = { .size = sizeof(cmd), .dev_id = device_id, .data_len = data_len, .data_uptr = (uint64_t)data, }; - struct iommu_test_hw_info *info = (struct iommu_test_hw_info *)data; int ret; ret = ioctl(fd, IOMMU_GET_HW_INFO, &cmd); @@ -389,8 +393,10 @@ static int _test_cmd_get_hw_info(int fd, __u32 device_id, } if (info) { - assert(info->test_reg == IOMMU_HW_INFO_SELFTEST_REGVAL); - assert(!info->flags); + if (data_len >= offsetofend(struct iommu_test_hw_info, test_reg)) + assert(info->test_reg == IOMMU_HW_INFO_SELFTEST_REGVAL); + if (data_len >= offsetofend(struct iommu_test_hw_info, flags)) + assert(!info->flags); } return 0;