On Thu, Jan 12, 2017 at 05:17:26PM -0200, Gustavo Padovan wrote: > 2017-01-10 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>: > > > Hi Daniel, > > > > 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 ... -Daniel -- 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
