Re: Why does OUT_FENCE_PTR point not to an fd (s32) but to an s64?

[Gustavo Padovan <gustavo@padovan.org>]

2017-01-10 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:

> 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.

Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel