1. 15 Mar, 2022 3 commits
    • Michel Dänzer's avatar
      xwayland: Clear timer_armed in xwl_present_unrealize_window · 987a34b6
      Michel Dänzer authored and Olivier Fourdan's avatar Olivier Fourdan committed
      Without this, xwl_present_reset_timer would call
      xwl_present_timer_callback if the timer was originally armed over a
      second ago. xwl_present_timer_callback would call xwl_present_msc_bump,
      which could end up hooking up the window to
      xwl_window->frame_callback_list again. This would lead to use-after-free
      in xwl_present_cleanup:
      
        Invalid write of size 8
          at 0x42B65C: __xorg_list_del (list.h:183)
          by 0x42B693: xorg_list_del (list.h:204)
          by 0x42C041: xwl_present_cleanup (xwayland-present.c:354)
          by 0x423669: xwl_destroy_window (xwayland-window.c:770)
          by 0x4FDDC5: compDestroyWindow (compwindow.c:620)
          by 0x5233FB: damageDestroyWindow (damage.c:1590)
          by 0x501C5F: DbeDestroyWindow (dbe.c:1326)
          by 0x4EF35B: FreeWindowResources (window.c:1018)
          by 0x4EF687: DeleteWindow (window.c:1086)
          by 0x4E24B3: doFreeResource (resource.c:885)
          by 0x4E2ED7: FreeClientResources (resource.c:1151)
          by 0x4ACBA4: CloseDownClient (...
      987a34b6
    • Olivier Fourdan's avatar
      xwayland/present: Fix use-after-free in xwl_unrealize_window() · fdbfda85
      Olivier Fourdan authored
      
      
      When a window is unrealized, Xwayland would destroy the Wayland surface
      prior to unrealizing the present window.
      
      xwl_present_flip() will then do a wl_surface_commit() of that surface,
      hence causing a use-after-free:
      
       Invalid read of size 8
          at 0x49F7FD4: wl_proxy_marshal_array_flags (wayland-client.c:852)
          by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
          by 0x42B877: wl_surface_commit (wayland-client-protocol.h:3914)
          by 0x42CAA7: xwl_present_flip (xwayland-present.c:717)
          by 0x42CD0E: xwl_present_execute (xwayland-present.c:783)
          by 0x42C26D: xwl_present_msc_bump (xwayland-present.c:416)
          by 0x42C2D1: xwl_present_timer_callback (xwayland-present.c:433)
          by 0x42BAC4: xwl_present_reset_timer (xwayland-present.c:149)
          by 0x42D1F8: xwl_present_unrealize_window (xwayland-present.c:945)
          by 0x4230E2: xwl_unrealize_window (xwayland-window.c:616)
          by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
          by 0x4F3F5C: UnrealizeTree (window.c:2805)
        Address 0x1390b8d8 is 24 bytes inside a block of size 80 free'd
          at 0x48470E4: free (vg_replace_malloc.c:872)
          by 0x49F8029: wl_proxy_destroy_caller_locks (wayland-client.c:523)
          by 0x49F8029: wl_proxy_marshal_array_flags (wayland-client.c:861)
          by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
          by 0x421984: wl_surface_destroy (wayland-client-protocol.h:3672)
          by 0x423052: xwl_unrealize_window (xwayland-window.c:599)
          by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
          by 0x4F3F5C: UnrealizeTree (window.c:2805)
          by 0x4F424B: UnmapWindow (window.c:2863)
          by 0x4EF58C: DeleteWindow (window.c:1075)
          by 0x4E24B3: doFreeResource (resource.c:885)
          by 0x4E2ED7: FreeClientResources (resource.c:1151)
          by 0x4ACBA4: CloseDownClient (dispatch.c:3546)
        Block was alloc'd at
          at 0x4849464: calloc (vg_replace_malloc.c:1328)
          by 0x49F7F29: zalloc (wayland-private.h:233)
          by 0x49F7F29: proxy_create (wayland-client.c:422)
          by 0x49F7F29: create_outgoing_proxy (wayland-client.c:664)
          by 0x49F7F29: wl_proxy_marshal_array_flags (wayland-client.c:831)
          by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
          by 0x4218CA: wl_compositor_create_surface (wayland-client-protocol.h:1291)
          by 0x422A0D: ensure_surface_for_window (xwayland-window.c:445)
          by 0x4231E8: xwl_window_set_window_pixmap (xwayland-window.c:647)
          by 0x5232D6: damageSetWindowPixmap (damage.c:1565)
          by 0x4FC7BC: compSetPixmapVisitWindow (compwindow.c:129)
          by 0x4EDB3F: TraverseTree (window.c:441)
          by 0x4FC851: compSetPixmap (compwindow.c:151)
          by 0x4F8C1A: compAllocPixmap (compalloc.c:616)
          by 0x4FC938: compCheckRedirect (compwindow.c:174)
      
      To avoid that, call xwl_present_unrealize_window() before destroying the
      Wayland surface.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit 42113ab2)
      fdbfda85
    • Olivier Fourdan's avatar
      Xwayland: Do not map the COW by default when rootless · 00530769
      Olivier Fourdan authored
      The composite overlay window (COW) can be queried from any X11 client,
      not just the X11 compositing manager.
      
      If a client tries to get the composite overlay window, the Xserver will
      map the window and block all pointer events (the window being mapped and
      on top of the stack).
      
      To avoid that issue, unset the "mapped" state of the composite overlay
      window once realized when Xwayland is running rootless.
      
      Note: All Xservers are actually affected by this issue, but with most
      regular X servers, the compositing manager will take care of dealing
      with the composite overlay window, and an X11 client using
      GetOverlayWindow() won't break pointer events for all X11 clients.
      Wayland compositors however usually run Xwayland rootless and have no
      use for the COW.
      
      v2: Avoid registering damage for the COW (Michel)
      v3: Remove the "mapped" test to avoid calling register_damage() if the
          COW is not mapped (Michel)
      
      Closes: #1314
      
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit 47d33174)
      00530769
  2. 11 Feb, 2022 2 commits
  3. 14 Dec, 2021 5 commits
  4. 02 Dec, 2021 4 commits
    • Olivier Fourdan's avatar
      xwayland/eglstream: Prefer EGLstream if available · bdc00ba7
      Olivier Fourdan authored
      
      
      Currently, when given the choice, Xwayland will pick the GBM backend
      over the EGLstream backend if both are available, unless the command
      line option “-eglstream” is specified.
      
      The NVIDIA proprietary driver had no support for GBM until driver series
      495, but starting with the driver series 495, both can be used.
      
      But there are other requirements with the rest of the stack, typically
      Mesa, egl-wayland, libglvnd as documented in the NVIDIA driver.
      
      So if the NVIDIA driver series 495 gets installed, Xwayland will pick
      the GBM backend even if EGLstream is available and may fail to render
      properly.
      
      To avoid that issue, prefer EGLstream if EGLstream and all the Wayland
      interfaces are available, and fallback to GBM automatically unless
      “-eglstream” was specified.
      
      With this, the compositor, given the choice, can decide which actual
      backend Xwayland would use by advertising (or not) the Wayland
      "wl_eglstream_controller" interface.
      
      This change has no impact on compositors which do not have support for
      EGLstream in the first place.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Acked-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit 6dd9709b)
      bdc00ba7
    • Olivier Fourdan's avatar
      xwayland/glamor: Log backend selected for debug · 3206e133
      Olivier Fourdan authored
      
      
      Add (verbose) statements to trace the actual backend used with glamor.
      
      That can be useful for debugging.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit c5d1fed9)
      3206e133
    • Olivier Fourdan's avatar
      xwayland/glamor: Change errors to verbose messages · a515f4f4
      Olivier Fourdan authored
      
      
      On a normal startup sequence, the Xwayland glamor backend would log
      an error whenever a required Wayland protocol is missing.
      
      Those are not really errors though, more informational messages along
      the glamor backend selection process.
      
      Demote those errors to verbose messages to reduce the verbosity of
      Xwayland at startup by default.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Jonas Ådahl's avatarJonas Ådahl <jadahl@gmail.com>
      (cherry picked from commit 30d0d4a1)
      a515f4f4
    • Olivier Fourdan's avatar
      xwayland/eglstream: Demote EGLstream device warning · 0a7ed9ff
      Olivier Fourdan authored
      
      
      If no EGLstream capable device is found at startup, Xwayland's EGLstream
      backend will log an error message "glamor: No eglstream capable devices
      found".
      
      However, considering that the vast majority of drivers do not implement
      EGLstream, the lack of EGLstream capable device is more of the norm than
      the exception.
      
      Change the error message to a log verbose message.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Simon Ser's avatarSimon Ser <contact@emersion.fr>
      Reviewed-by: Jonas Ådahl's avatarJonas Ådahl <jadahl@gmail.com>
      (cherry picked from commit 96c82bef)
      0a7ed9ff
  5. 17 Nov, 2021 1 commit
  6. 08 Nov, 2021 1 commit
  7. 21 Oct, 2021 2 commits
  8. 20 Oct, 2021 1 commit
  9. 18 Oct, 2021 5 commits
  10. 09 Jul, 2021 3 commits
  11. 30 Jun, 2021 3 commits
  12. 21 Jun, 2021 10 commits
    • Olivier Fourdan's avatar
      glx: Set ContextTag for all contexts · 763f4fb2
      Olivier Fourdan authored
      
      
      Currently, xorgGlxMakeCurrent() would set the context tag only for
      indirect GLX contexts.
      
      However, several other places expect to find a context for the tag or
      they would raise a GLXBadContextTag error, such as WaitGL() or WaitX().
      
      Set the context tag for direct contexts as well, to avoid raising an
      error and possibly killing the client and set currentClient.
      
      Thanks to Erik Kurzinger <ekurzinger@nvidia.com> for spotting the issue.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Adam Jackson's avatarAdam Jackson <ajax@redhat.com>
      (cherry picked from commit c468d34c)
      (cherry picked from commit aad61e8e)
      763f4fb2
    • Erik Kurzinger's avatar
      glx: don't create implicit GLXWindow if one already exists · e754b473
      Erik Kurzinger authored and Olivier Fourdan's avatar Olivier Fourdan committed
      
      
      If a GLXMakeCurrent request specifies an X window as its drawable,
      __glXGetDrawable will implicitly create a GLXWindow for it. However,
      the client may have already explicitly created a GLXWindow for that X
      window. If that happens, two __glXDrawableRes resources will be added
      to the window.
      
      If the explicitly-created GLXWindow is later destroyed by the client,
      DrawableGone will call FreeResourceByType on the X window, but this
      will actually free the resource for the implicitly-created GLXWindow,
      since that one would be at the head of the list.
      
      Then if the X window is destroyed after that, the resource for the
      explicitly-created GLXWindow will be freed. But that GLXWindow was
      already destroyed above. This crashes the server when it tries to call
      the destroyed GLXWindow's destructor. It also means the
      implicitly-created GLXWindow would have been leaked since the
      FreeResourceByType call mentioned above skips calling the destructor.
      
      To fix this, if __glXGetDrawable is given an X window, it should check
      if there is already a GLXWindow associated with it, and only create an
      implicit one if there is not.
      Signed-off-by: Erik Kurzinger's avatarErik Kurzinger <ekurzinger@nvidia.com>
      Reviewed-by: Adam Jackson's avatarAdam Jackson <ajax@redhat.com>
      (cherry picked from commit b7a85e44)
      e754b473
    • Olivier Fourdan's avatar
      xwayland/eglstream: Log when GL_OES_EGL_image is missing · 831426af
      Olivier Fourdan authored
      
      
      That will dramatically affect performance, might as well log when we
      cannot use GL_OES_EGL_image with the NVIDIA closed-source driver.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit 34a58d77)
      831426af
    • Olivier Fourdan's avatar
      xwayland/eglstream: Use "nvidia" for GLVND · 2e1bb506
      Olivier Fourdan authored
      
      
      If the EGLStream backend is able to use hardware acceleration with the
      NVIDIA closed source driver, we should use the "nvidia" GLX
      implementation instead of the one from Mesa to take advantage of the
      NVIDIA hardware accelerated rendering.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit fae58e9b)
      2e1bb506
    • Olivier Fourdan's avatar
      xwayland: Add preferred GLVND vendor to xwl_screen · 8744ab1b
      Olivier Fourdan authored
      
      
      If Xwayland's EGLstream backend supports hardware acceleration with the
      NVIDIA closed-source driver, the GLX library also needs to be one
      shipped by NVIDIA, that's what GLVND is for.
      
      Add a new member to the xwl_screen that the backend can optionally set
      to the preferred GLVND vendor to use.
      
      If not set, "mesa" is assumed.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit 24fc8aea)
      8744ab1b
    • Erik Kurzinger's avatar
      xwayland/eglstream: flush stream after eglSwapBuffers · f4720f1c
      Erik Kurzinger authored and Olivier Fourdan's avatar Olivier Fourdan committed
      When eglSwapBuffers inserts a new frame into a window's stream, there may be a
      delay before the state of the consumer end of the stream is updated to reflect
      this. If the subsequent wl_surface_attach, wl_surface_damage, wl_surface_commit
      calls are received by the compositor before then, it will (typically) re-use
      the previous frame acquired from the stream instead of the latest one.
      
      This can leave the window displaying out-of-date contents, which might never be
      updated thereafter.
      
      To fix this, after calling eglSwapBuffers, xwl_glamor_eglstream_post_damage
      should call eglStreamFlushNV. This call will block until it can be guaranteed
      that the state of the consumer end of the stream has been updated to reflect
      that a new frame is available.
      
      Closes: #1171
      
      Signed-off-by: Erik Kurzinger's avatarErik Kurzinger <ekurzinger@nvidia.com>
      (cherry picked from commit 7515c23a)
      f4720f1c
    • Erik Kurzinger's avatar
      xwayland/eglstream: allow commits to dma-buf backed pixmaps · b2963fcc
      Erik Kurzinger authored and Olivier Fourdan's avatar Olivier Fourdan committed
      As of commit 098e0f52
      
       xwl_glamor_eglstream_allow_commits will not allow commits
      if the xwl_pixmap does not have an EGLSurface. This is valid for pixmaps backed
      by an EGLStream, however pixmaps backed by a dma-buf for OpenGL or Vulkan
      rendering will never have an EGLSurface.  Unlike EGLStream backed pixmaps,
      though, glamor will render directly to the buffer that Xwayland passes to the
      compositor. Hence, they don't require the intermediate copy in
      xwl_glamor_eglstream_post_damage that EGLStream backed pixmaps do, so there is
      no need for an EGLSurface.
      Signed-off-by: Erik Kurzinger's avatarErik Kurzinger <ekurzinger@nvidia.com>
      Acked-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit 3d33d885)
      b2963fcc
    • Olivier Fourdan's avatar
      xwayland/eglstream: Set ALU to GXCopy for blitting · bdb4712d
      Olivier Fourdan authored
      
      
      The EGLstream backend's post damage function uses a shader and
      glDrawArrays() to copy the data from the glamor's pixmap texture prior
      to do the eglSwapBuffers().
      
      However, glDrawArrays() can be affected by the GL state, and therefore
      not reliably produce the expected copy, causing the content of the
      buffer to be corrupted.
      
      Make sure to set the ALU to GXCopy prior to call glDrawArrays() to get
      the expected result.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Suggested-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit 012350e3)
      bdb4712d
    • Olivier Fourdan's avatar
      xwayland/eglstream: Do not always increment pixmap refcnt on commit · 96febe8b
      Olivier Fourdan authored
      
      
      Currently, the EGLstream backend would increment the pixmap refcount for
      each commit, and decrease that refcount on the wl_buffer release
      callback.
      
      But that's relying on the compositor sending us a release callback for
      each commit, otherwise the pixmap refcount will keep increasing and the
      pixmap will be leaked.
      
      So instead, increment the refcount on the pixmap only when we have not
      received a release notification for the wl_buffer, to avoid increasing
      the pixmap refcount more than once without a corresponding release
      event.
      
      This way, if the pixmap is still in use when released on the X11 side,
      the EGL stream will be kept until the compositor releases it.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Suggested-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit d85bfa6a)
      96febe8b
    • Olivier Fourdan's avatar
      xwayland/eglstream: Check eglSwapBuffers() · 7d839b3e
      Olivier Fourdan authored
      
      
      EGLstream's post_damage() would unconditionally return success
      regardless of the actual status of the eglSwapBuffers().
      
      Yet, if eglSwapBuffers() fails, we should not post the corresponding
      damage as they wouldn't match the actual content of the buffer.
      
      Use the eglSwapBuffers() return value as the return value for
      post_damage() and do not take a refrence on the pixmap if it fails.
      Signed-off-by: Olivier Fourdan's avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: Michel Dänzer's avatarMichel Dänzer <mdaenzer@redhat.com>
      (cherry picked from commit b583395c)
      7d839b3e