| Message ID | 20240729024604.2046-1-yuzenghui@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kselftests: dmabuf-heaps: Ensure the driver name is null-terminated | expand |
On Mon, Jul 29, 2024 at 10:46:04AM +0800, Zenghui Yu wrote: > Even if a vgem device is configured in, we will skip the import_vgem_fd() > test almost every time. > > TAP version 13 > 1..11 > # Testing heap: system > # ======================================= > # Testing allocation and importing: > ok 1 # SKIP Could not open vgem -1 > > The problem is that we use the DRM_IOCTL_VERSION ioctl to query the driver > version information but leave the name field a non-null-terminated string. > Terminate it properly to actually test against the vgem device. > > While at it, let's check the length of the driver name is exactly 4 bytes > and return early otherwise (in case there is a name like "vgemfoo" that > gets converted to "vgem\0" unexpectedly). > > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> > --- > * From v1 [1]: > - Check version.name_len is exactly 4 bytes and return early otherwise > > [1] https://lore.kernel.org/r/20240708134654.1725-1-yuzenghui@huawei.com Thanks for your patch, I'll push it to drm-misc-next-fixes. > P.S., Maybe worth including the kselftests file into "DMA-BUF HEAPS > FRAMEWORK" MAINTAINERS entry? Good idea, want to do the patch for that too? Cheers, Sima > > tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c > index 5f541522364f..5d0a809dc2df 100644 > --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c > +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c > @@ -29,9 +29,11 @@ static int check_vgem(int fd) > version.name = name; > > ret = ioctl(fd, DRM_IOCTL_VERSION, &version); > - if (ret) > + if (ret || version.name_len != 4) > return 0; > > + name[4] = '\0'; > + > return !strcmp(name, "vgem"); > } > > -- > 2.33.0 >
On 2024/7/29 15:01, Daniel Vetter wrote: > On Mon, Jul 29, 2024 at 10:46:04AM +0800, Zenghui Yu wrote: > > Even if a vgem device is configured in, we will skip the import_vgem_fd() > > test almost every time. > > > > TAP version 13 > > 1..11 > > # Testing heap: system > > # ======================================= > > # Testing allocation and importing: > > ok 1 # SKIP Could not open vgem -1 > > > > The problem is that we use the DRM_IOCTL_VERSION ioctl to query the driver > > version information but leave the name field a non-null-terminated string. > > Terminate it properly to actually test against the vgem device. > > > > While at it, let's check the length of the driver name is exactly 4 bytes > > and return early otherwise (in case there is a name like "vgemfoo" that > > gets converted to "vgem\0" unexpectedly). > > > > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> > > --- > > * From v1 [1]: > > - Check version.name_len is exactly 4 bytes and return early otherwise > > > > [1] https://lore.kernel.org/r/20240708134654.1725-1-yuzenghui@huawei.com > > Thanks for your patch, I'll push it to drm-misc-next-fixes. > > > P.S., Maybe worth including the kselftests file into "DMA-BUF HEAPS > > FRAMEWORK" MAINTAINERS entry? > > Good idea, want to do the patch for that too? Sure, I can send a patch for that today. Thanks, Zenghui
diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c index 5f541522364f..5d0a809dc2df 100644 --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c @@ -29,9 +29,11 @@ static int check_vgem(int fd) version.name = name; ret = ioctl(fd, DRM_IOCTL_VERSION, &version); - if (ret) + if (ret || version.name_len != 4) return 0; + name[4] = '\0'; + return !strcmp(name, "vgem"); }
Even if a vgem device is configured in, we will skip the import_vgem_fd() test almost every time. TAP version 13 1..11 # Testing heap: system # ======================================= # Testing allocation and importing: ok 1 # SKIP Could not open vgem -1 The problem is that we use the DRM_IOCTL_VERSION ioctl to query the driver version information but leave the name field a non-null-terminated string. Terminate it properly to actually test against the vgem device. While at it, let's check the length of the driver name is exactly 4 bytes and return early otherwise (in case there is a name like "vgemfoo" that gets converted to "vgem\0" unexpectedly). Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> --- * From v1 [1]: - Check version.name_len is exactly 4 bytes and return early otherwise [1] https://lore.kernel.org/r/20240708134654.1725-1-yuzenghui@huawei.com P.S., Maybe worth including the kselftests file into "DMA-BUF HEAPS FRAMEWORK" MAINTAINERS entry? tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)