On Thu, Aug 17, 2023 at 07:32:52AM +0000, Tian, Kevin wrote:
+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 info;uint64_t trailing_bytes;} buffer_larger;if (self->device_id) {/* Provide a zero-size user_buffer */test_cmd_get_hw_info(self->device_id, NULL, 0);/* Provide a user_buffer with exact size */test_cmd_get_hw_info(self->device_id, &buffer_exact,sizeof(buffer_exact));
/** Provide a user_buffer with size larger than the exact size tocheck if
* kernel zero the trailing bytes.*/test_cmd_get_hw_info(self->device_id, &buffer_larger,sizeof(buffer_larger));
Do we also want to test a case where size is smaller than the exact size?
Added one with the following diff:
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;