On Fri, Jan 06, 2017 at 01:04:55PM -0800, Chad Versace wrote:
> Was this a mistake in the API? If so, can we fix this ABI mistake before
> kernel consumers rely on this?
>
> I naïvely expected that OUT_FENCE_PTR would be a pointer to, obviously, a fence
> fd (s32 __user *). But it's not. It's s64 __user *. Due to that surprise, I
> spent several hours chasing down weird corruption in Rob Clark's kmscube. The
> kernel unexpectedly cleared the 32 bits *above* an `int kms_fence_fd` in
> userspace.
Never use unsized types for uabi. I guess we could have used s32, but then
someone is going to store this in a long and it goes boom on 64 bit, while
it works on 32 bit. "int" doesn't have that problem directly, but it's
still a red flag imo.
-Daniel
>
> For reference, here's the relevant DRM code.
>
> // file: drivers/gpu/drm/drm_atomic.c
> struct drm_out_fence_state {
> s64 __user *out_fence_ptr;
> struct sync_file *sync_file;
> int fd;
> };
>
> static int setup_out_fence(struct drm_out_fence_state *fence_state,
> struct dma_fence *fence)
> {
> fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> if (fence_state->fd < 0)
> return fence_state->fd;
>
> if (put_user(fence_state->fd, fence_state->out_fence_ptr))
> return -EFAULT;
>
> fence_state->sync_file = sync_file_create(fence);
> if (!fence_state->sync_file)
> return -ENOMEM;
>
> return 0;
> }
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
