Recognizing wl_display pointer for EGL (Re: Smart comparing of wl_display_interfaces)

[Jason Ekstrand]

On Thu, Oct 16, 2014 at 11:16 PM, Pekka Paalanen <ppaalanen at gmail.com>
wrote:

> On Thu, 16 Oct 2014 10:40:20 +0300
> Pekka Paalanen <ppaalanen at gmail.com> wrote:
>
> > On Thu, 16 Oct 2014 09:46:39 +0300
> > Pekka Paalanen <ppaalanen at gmail.com> wrote:
> >
> > > On Wed, 15 Oct 2014 22:44:25 +0400
> > > Dmitry Cherkassov <dcherkassov at gmail.com> wrote:
> > >
> > > > Hi list!
> > > >
> > > > The definition of wl_display_interface symbol can come from
> > > > libwayland-server.so or libwayland-client.so.
> > > >
> > > > There is a code in closed source EGL implementation that does
> > > > interfaces comparison via pointers, yielding negative results when
> > > > addresses are different (one address is saved by QtWayland, the other
> > > > one is from &wl_display_interface in EGL source code).
> > > >
> > > > Is there a smarter way to compare wl_display_interface's?
> > > >
> > > > There is this function:
> > > >
> > > > wl_interface_equal(const struct wl_interface *a, const struct
> wl_interface *b)
> > > > {
> > > >     return a == b || strcmp(a->name, b->name) == 0;
> > > > }
> > > >
> > > > But as you can see it's not safe to use it alone because in case of
> > > > bad pointer we will get undefined results.
> > > >
> > > > Currently struct wl_interface is the following:
> > > >  struct wl_interface {
> > > >      const char *name;
> > > >      int version;
> > > >      int method_count;
> > > >      const struct wl_message *methods;
> > > >      int event_count;
> > > >      const struct wl_message *events;
> > > >  };
> > > >
> > > > Since there is no magic field at the beginning it is not possible to
> > > > see whether a pointer points to the valid wl_interface structure.
> > > >
> > > > So my question is: how can we compare wl_interfaces in more clever
> and
> > > > safer way?
> > >
> > > Hmm, why would you ever have a bad pointer?
> > >
> > > Bad pointers in general tend to cause crashes and stuff, so you
> > > shouldn't be having a bad one in the first place.
> > >
> > > What is the background for your question/problem?
> >
> > Ok, I suppose this was discussed in IRC last night: the actual problem
> > behind the question is how to detect whether the void pointer
> > (EGLNativeDisplayType) given to eglCreateDisplay is actually a
> > wl_display.
> >
> > That's tough.
> >
> > A couple of hacks come to mind.
> >
> > bool
> > _eglNativePlatformDetectWayland(void *nativeDisplay)
> > {
> >       void *f;
> >
> >       if (!dereferencable(nativeDisplay))
> >               return false;
> >
> >       f = *(void **)nativeDisplay;
> >
> > The first is to dlopen both libwayland-client and libwayland-server in
> > turn with RTLD_NOLOAD | RTLD_LOCAL, and fetching the
> > wl_display_interface symbol from each, comparing those to 'f'. If
> > either matches, return true.
>
> While discussing in IRC, I realized half of this is nonsense.
>
> eglGetDisplay() can only get a wl_display created by libwayland-client,
> and never one created by libwayland-server. Assuming the dynamic
> linking is sane, the interface pointer in wl_display is *always* the
> one from libwayland-client.so.
>
> Now we just need to make sure, that we compare 'f' to the interface
> pointer from libwayland-client.so. That could probably be done with the
> dlopen/dlsym trick, or...
>
> > Another would be to check if 'f' is dereferencable (and not equal to
> > the default wl_display_interface), and check if (char *)f starts with
> > "wl_d". If yes, do a full string comparison with "wl_display". It's
> > not reliable, but maybe works enough.
> >
> > The proper solution is for apps to use EGL_KHR_platform_wayland to
> > explicitly say the pointer is a wl_display. That of course doesn't work
> > on old apps, so you might implement a workaround through environment
> > like Mesa does: EGL_PLATFORM=wayland bypasses all autodetection and
> > assumes it is a wl_display.
> >
> > In any case, EGL really should implement EGL_KHR_platform_wayland so we
> > have a chance to get rid of the inherently unreliable autodetect. And we
> > should start using that in our demo apps...
> >
> > If we consider autodetecting client wl_display pointers worth to solve
> > properly, we should probably add a function for that in
> > libwayland-client.so. However, implementing that portably might be
> > difficult.
>
> ...adding a function int/bool wl_is_wl_display_pointer(void *p) into
> libwayland-client.so. As that function would be built into
> libwayland-client.so, it naturally uses the client.so version of
> wl_display_interface symbol (unless dynamic linking miraculously
> manages to mess that up?).
>
> 'p' would be the tentative 'struct wl_display *', so that the
> implementation details of struct wl_display would be contained in
> libwayland-client.so, and not leaked elsewhere like e.g. in Mesa.
>
> All this means we should be able to get away with just one pointer
> comparison, after assuring that the first sizeof(void*) bytes pointed
> to by the given unknown pointer (EGLNativeDisplayType) are
> dereferencable without a segfault or other fatal error.
>
> So, a quick fix for EGL implementations right now would be to try the
> dlopen hack, until they can start to depend on a new
> libwayland-client.so providing wl_is_wl_display_pointer().
>
> I am unsure about the exact definition of wl_is_wl_display_pointer():
> can it do the dereferencability check itself, or should we require the
> caller to guarantee that dereferencing cannot crash. The latter would
> leave the portability burden on the callers rather than libwayland.
>
> How's that sound?
>

That sounds fine.  I think we can count on the client passing in a valid
pointer to something.  Chances are that whatever the client passes in will
be a pointer to some sort of structure and so we can probably read 8 bytes
without a problem.  If we really wanted to, we could do a quick alignment
check to ensure that it's sizeof(void *)-aligned.

I don't know that I care whether we do your wl_is_wl_display_pointer or
Bill's "dynamic cast" version.  It doesn't much matter.

Is it possible to have several instances of libwayland-client in a
> process address space? Meaning we would have several different
> client-side wl_display_interface pointers?
>

I don't think so.  At least not without some very bizarre hackery.  Also, I
think it's reasonable to assume that either a) libEGL is reasonably
normally linked against the same chunks of code that link to
libwayland-client or b) The client really knows what they're doing and is
prepared for the consequences.  Because really, if they're pulling those
kinds of tricks they're asking for trouble.


> If yes, it's back to the string comparisons...
>

String comparisons are right out.  They'll cause a segfault the moment it's
not a wl_display pointer.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20141018/a3d9d222/attachment-0001.html>