Hi Daniel, On Thursday 12 Jan 2017 20:26:40 Daniel Vetter wrote: > On Thu, Jan 12, 2017 at 05:17:26PM -0200, Gustavo Padovan wrote: > > 2017-01-10 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>: > >> On Monday 09 Jan 2017 11:23:23 Daniel Vetter wrote: > >>> 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, > >> > >> Why so ? And why do we care ? The commonly accepted practice is to store > >> file descriptors in int variables. s32 is an int on all platforms, so > >> that's fine too. If we use a s32 pointer here, and someone decides to > >> store it in a long, bool or cast it to a complex, that's their problem > >> :-) > > > > The only thing that really needs to be s64 here is the OUT_FENCE_PTR > > property in the Atomic interface because we carry a pointer there, but > > all the manipulation after that is actually done after can easily be > > done on s32 or int. > > > > We can't expect that userspace will know that we store as s64 and clear > > the bits above if a int was passed down. if we use s32 we will be in > > complaince with other linux apis that deals with fds. I'd say we fix > > this before it can cause more damage out there. > > Changing uabi is kinda tricky, but it's still very new, so if we make sure > it gets applied everywhere and doesn't accidentally ship we can it. Iirc > fences are only in 4.10, so we should be fine ... Correct. That sounds good to me, there's still time for a v4.10-rc fix. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
