On 3/1/26 13:34, Julian Orth wrote:
> 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]>
> ---
> This patch fixes a regression that would cause conversions between
> syncobj handles and fds to fail if userspace did not initialize a
> recently-added field to 0.
While I don't object to this change, I'd argue that this is really a workaround
for broken user space, not a fix for a kernel regression.
User space must initialize the full struct to 0, except for the fields it wants
to have other values. This kind of kernel-side workaround for neglecting that
won't be possible in all cases.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
