Hey,
Den 2026-03-01 kl. 13:34, skrev Julian Orth:
> Consider the following application:
>
> #include <fcntl.h>
> #include <string.h>
> #include <drm/drm.h>
> #include <sys/ioctl.h>
>
> int main(void) {
> int fd = open("/dev/dri/renderD128", O_RDWR);
> struct drm_syncobj_create arg1;
> ioctl(fd, DRM_IOCTL_SYNCOBJ_CREATE, &arg1);
> struct drm_syncobj_handle arg2;
> memset(&arg2, 1, sizeof(arg2)); // simulate dirty stack
> arg2.handle = arg1.handle;
> arg2.flags = 0;
> arg2.fd = 0;
> arg2.pad = 0;
> // arg2.point = 0; // userspace is required to set point to 0
> ioctl(fd, DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD, &arg2);
> }
>
> The last ioctl returns EINVAL because args->point is not 0. However,
> userspace developed against older kernel versions is not aware of the
> new point field and might therefore not initialize it.
>
> The correct check would be
>
> if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE)
> return -EINVAL;
>
> However, there might already be userspace that relies on this not
> returning an error as long as point == 0. Therefore use the more lenient
> check.
>
> Fixes: c2d3a7300695 ("drm/syncobj: Extend EXPORT_SYNC_FILE for timeline
> syncobjs")
> Signed-off-by: Julian Orth <[email protected]>
I'm not convinced this is the correct fix.
Userspace built before the change had the old size for drm_syncobj_create,
the size is encoded into the ioctl, and zero extended as needed.
See drivers/gpu/drm/drm_ioctl.c:
out_size = in_size = _IOC_SIZE(cmd);
...
if (ksize > in_size)
memset(kdata + in_size, 0, ksize - in_size);
This is a bug in a newly built app, and should be handled by explicitly zeroing
the entire struct or using named initializers, and only setting specific members
as required.
In particular, apps built before the change will never encounter this bug.
Kind regards,
~Maarten Lankhorst
