Re: [RFC 6/9] drm/i915: Add sync framework support to execbuff IOCTL

Chris Wilson
2016-01-21T15:08:02Z

On Thu, Jan 21, 2016 at 03:47:40PM +0100, Maarten Lankhorst wrote:
> Op 14-01-16 om 13:07 schreef Chris Wilson:
> > On Thu, Jan 14, 2016 at 11:47:17AM +0000, John Harrison wrote:
> >> On 13/01/2016 18:43, Chris Wilson wrote:
> >>> Use the upper s32 for the output, so again you are not overwriting user
> >>> state without good reason.
> >>>
> >> Makes sense. Will do.
> > It would also be useful (for nefarious reasons) to only copy back the
> > rsvd2 field, i.e. keep it as DRM_IOW(EXECBUFFER2) and do the small
> > fixed-sized copy_to_user of rsvd2 as required. That means we can avoid
> > the copy mostly, and treat args as a private copy of the user
> > parameters, i.e. space we can scribble over with impunity.
> Won't that be an abuse of abi? If we write memory we should mark the struct as writable, and make sure we don't touch the user execbuffer2 at all until the call succeeds.

We only write into the structure when the user requests an output
parameter, so they have to opt in (because the current ABI has no
outputs through the ioctl atm). Whether or not userspace marks the ioctl
argument as RW/RO is irrelevant to the kernel who uses its own table to
control the ABI. If the memory really is read-only, then copy_to_user()
will -EFAULT (and as the user requested an output we should check early
and fail, whether to fail after we dispatch the request is an interesting
conundrum).
-Chris

--
Chris Wilson, Intel Open Source Technology Centre