From e9feaefa4bd3557556dbbbc21b41bd685ab76a5a Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Mon, 24 Apr 2023 11:05:34 +0300 Subject: [PATCH] Handling failing to export GEM handle to prime fd If gbm_bo_get_fd_for_plane() or gbm_bo_get_fd() fails, ensure that the gbm buffer allocator properly handles it and doesn't return a GraphicsBuffer object. --- src/backends/drm/drm_backend.cpp | 11 ++++++++--- src/backends/drm/drm_egl_backend.cpp | 12 ++++++++++-- src/backends/drm/drm_egl_layer_surface.cpp | 10 +++++++--- src/backends/drm/gbm_dmabuf.h | 13 ++++++++++++- src/backends/virtual/virtual_egl_backend.cpp | 4 ++++ src/backends/wayland/wayland_backend.cpp | 9 +++++++-- src/backends/wayland/wayland_egl_backend.cpp | 3 +++ .../x11/windowed/x11_windowed_egl_backend.cpp | 3 +++ src/core/gbmgraphicsbufferallocator.cpp | 16 +++++++++++----- src/core/gbmgraphicsbufferallocator.h | 2 +- 10 files changed, 66 insertions(+), 17 deletions(-) diff --git a/src/backends/drm/drm_backend.cpp b/src/backends/drm/drm_backend.cpp index b91bf6897e..a3c8690974 100644 --- a/src/backends/drm/drm_backend.cpp +++ b/src/backends/drm/drm_backend.cpp @@ -435,12 +435,17 @@ std::shared_ptr DrmBackend::createDmaBufTexture(const QSize &size } // The bo will be kept around until the last fd is closed. - DmaBufAttributes attributes = dmaBufAttributesForBo(bo); + std::optional attributes = dmaBufAttributesForBo(bo); gbm_bo_destroy(bo); + + if (!attributes.has_value()) { + return nullptr; + } + const auto eglBackend = static_cast(m_renderBackend); eglBackend->makeCurrent(); - if (auto texture = eglBackend->importDmaBufAsTexture(attributes)) { - return std::make_shared(texture, std::move(attributes)); + if (auto texture = eglBackend->importDmaBufAsTexture(attributes.value())) { + return std::make_shared(texture, std::move(attributes.value())); } else { return nullptr; } diff --git a/src/backends/drm/drm_egl_backend.cpp b/src/backends/drm/drm_egl_backend.cpp index bfb2217de1..ef44a84e2a 100644 --- a/src/backends/drm/drm_egl_backend.cpp +++ b/src/backends/drm/drm_egl_backend.cpp @@ -283,12 +283,20 @@ bool operator==(const GbmFormat &lhs, const GbmFormat &rhs) EGLImageKHR EglGbmBackend::importBufferObjectAsImage(gbm_bo *bo) { - return importDmaBufAsImage(dmaBufAttributesForBo(bo)); + std::optional attributes = dmaBufAttributesForBo(bo); + if (!attributes.has_value()) { + return EGL_NO_IMAGE_KHR; + } + return importDmaBufAsImage(attributes.value()); } std::shared_ptr EglGbmBackend::importBufferObjectAsTexture(gbm_bo *bo) { - return importDmaBufAsTexture(dmaBufAttributesForBo(bo)); + std::optional attributes = dmaBufAttributesForBo(bo); + if (!attributes.has_value()) { + return nullptr; + } + return importDmaBufAsTexture(attributes.value()); } } // namespace KWin diff --git a/src/backends/drm/drm_egl_layer_surface.cpp b/src/backends/drm/drm_egl_layer_surface.cpp index 33ab840d3e..215c9c9942 100644 --- a/src/backends/drm/drm_egl_layer_surface.cpp +++ b/src/backends/drm/drm_egl_layer_surface.cpp @@ -75,7 +75,7 @@ std::optional EglGbmLayerSurface::startRendering(cons } auto &[texture, fbo] = m_surface.textureCache[buffer->bo()]; if (!texture) { - texture = m_eglBackend->importDmaBufAsTexture(dmaBufAttributesForBo(buffer->bo())); + texture = m_eglBackend->importBufferObjectAsTexture(buffer->bo()); if (!texture) { return std::nullopt; } @@ -355,7 +355,9 @@ std::shared_ptr EglGbmLayerSurface::importWithEgl(Surface &surfa context->makeCurrent(); auto &sourceTexture = surface.importedTextureCache[sourceBuffer->bo()]; if (!sourceTexture) { - sourceTexture = context->importDmaBufAsTexture(dmaBufAttributesForBo(sourceBuffer->bo())); + if (std::optional attributes = dmaBufAttributesForBo(sourceBuffer->bo())) { + sourceTexture = context->importDmaBufAsTexture(attributes.value()); + } } if (!sourceTexture) { qCWarning(KWIN_DRM, "failed to import the source texture!"); @@ -364,7 +366,9 @@ std::shared_ptr EglGbmLayerSurface::importWithEgl(Surface &surfa const auto [localBuffer, repaint] = surface.importGbmSwapchain->acquire(); auto &[texture, fbo] = surface.importTextureCache[localBuffer->bo()]; if (!texture) { - texture = context->importDmaBufAsTexture(dmaBufAttributesForBo(localBuffer->bo())); + if (std::optional attributes = dmaBufAttributesForBo(localBuffer->bo())) { + texture = context->importDmaBufAsTexture(attributes.value()); + } if (!texture) { qCWarning(KWIN_DRM, "failed to import the local texture!"); return nullptr; diff --git a/src/backends/drm/gbm_dmabuf.h b/src/backends/drm/gbm_dmabuf.h index c9a767acea..102cce5aaf 100644 --- a/src/backends/drm/gbm_dmabuf.h +++ b/src/backends/drm/gbm_dmabuf.h @@ -13,11 +13,14 @@ #include #include +#include + +#include namespace KWin { -inline DmaBufAttributes dmaBufAttributesForBo(gbm_bo *bo) +inline std::optional dmaBufAttributesForBo(gbm_bo *bo) { DmaBufAttributes attributes; attributes.planeCount = gbm_bo_get_plane_count(bo); @@ -29,6 +32,10 @@ inline DmaBufAttributes dmaBufAttributesForBo(gbm_bo *bo) #if HAVE_GBM_BO_GET_FD_FOR_PLANE for (int i = 0; i < attributes.planeCount; ++i) { attributes.fd[i] = FileDescriptor{gbm_bo_get_fd_for_plane(bo, i)}; + if (!attributes.fd[i].isValid()) { + qWarning() << "gbm_bo_get_fd_for_plane() failed:" << strerror(errno); + return std::nullopt; + } attributes.offset[i] = gbm_bo_get_offset(bo, i); attributes.pitch[i] = gbm_bo_get_stride_for_plane(bo, i); } @@ -38,6 +45,10 @@ inline DmaBufAttributes dmaBufAttributesForBo(gbm_bo *bo) } attributes.fd[0] = FileDescriptor{gbm_bo_get_fd(bo)}; + if (!attributes.fd[0].isValid()) { + qWarning() << "gbm_bo_get_fd() failed:" << strerror(errno); + return std::nullopt; + } attributes.offset[0] = gbm_bo_get_offset(bo, 0); attributes.pitch[0] = gbm_bo_get_stride_for_plane(bo, 0); #endif diff --git a/src/backends/virtual/virtual_egl_backend.cpp b/src/backends/virtual/virtual_egl_backend.cpp index 69363f1873..c8eb3f85c4 100644 --- a/src/backends/virtual/virtual_egl_backend.cpp +++ b/src/backends/virtual/virtual_egl_backend.cpp @@ -104,6 +104,10 @@ std::optional VirtualEglLayer::beginFrame() } m_current = m_swapchain->acquire(); + if (!m_current) { + return std::nullopt; + } + return OutputLayerBeginFrameInfo{ .renderTarget = RenderTarget(m_current->framebuffer()), .repaint = infiniteRegion(), diff --git a/src/backends/wayland/wayland_backend.cpp b/src/backends/wayland/wayland_backend.cpp index 72490474c2..abafe3fc3f 100644 --- a/src/backends/wayland/wayland_backend.cpp +++ b/src/backends/wayland/wayland_backend.cpp @@ -614,10 +614,15 @@ std::shared_ptr WaylandBackend::createDmaBufTexture(const QSize & } // The bo will be kept around until the last fd is closed. - DmaBufAttributes attributes = dmaBufAttributesForBo(bo); + std::optional attributes = dmaBufAttributesForBo(bo); gbm_bo_destroy(bo); + + if (!attributes.has_value()) { + return nullptr; + } + m_eglBackend->makeCurrent(); - return std::make_shared(m_eglBackend->importDmaBufAsTexture(attributes), std::move(attributes)); + return std::make_shared(m_eglBackend->importDmaBufAsTexture(attributes.value()), std::move(attributes.value())); } void WaylandBackend::setEglDisplay(std::unique_ptr &&display) diff --git a/src/backends/wayland/wayland_egl_backend.cpp b/src/backends/wayland/wayland_egl_backend.cpp index a1d9e42dc2..0f0894d281 100644 --- a/src/backends/wayland/wayland_egl_backend.cpp +++ b/src/backends/wayland/wayland_egl_backend.cpp @@ -174,6 +174,9 @@ std::optional WaylandEglPrimaryLayer::beginFrame() } m_buffer = m_swapchain->acquire(); + if (!m_buffer) { + return std::nullopt; + } QRegion repair; if (m_backend->supportsBufferAge()) { diff --git a/src/backends/x11/windowed/x11_windowed_egl_backend.cpp b/src/backends/x11/windowed/x11_windowed_egl_backend.cpp index 54af7f4ccf..624bff020b 100644 --- a/src/backends/x11/windowed/x11_windowed_egl_backend.cpp +++ b/src/backends/x11/windowed/x11_windowed_egl_backend.cpp @@ -147,6 +147,9 @@ std::optional X11WindowedEglPrimaryLayer::beginFrame( } m_buffer = m_swapchain->acquire(); + if (!m_buffer) { + return std::nullopt; + } QRegion repaint = m_output->exposedArea() + m_output->rect(); m_output->clearExposedArea(); diff --git a/src/core/gbmgraphicsbufferallocator.cpp b/src/core/gbmgraphicsbufferallocator.cpp index 9ebb4d58ac..5ee1f0bf47 100644 --- a/src/core/gbmgraphicsbufferallocator.cpp +++ b/src/core/gbmgraphicsbufferallocator.cpp @@ -47,7 +47,13 @@ GbmGraphicsBuffer *GbmGraphicsBufferAllocator::allocate(const QSize &size, uint3 return nullptr; } - return new GbmGraphicsBuffer(bo, size, format); + std::optional attributes = dmaBufAttributesForBo(bo); + if (!attributes.has_value()) { + gbm_bo_destroy(bo); + return nullptr; + } + + return new GbmGraphicsBuffer(std::move(attributes.value()), bo); } static bool alphaChannelFromDrmFormat(uint32_t drmFormat) @@ -87,11 +93,11 @@ static bool alphaChannelFromDrmFormat(uint32_t drmFormat) } } -GbmGraphicsBuffer::GbmGraphicsBuffer(gbm_bo *handle, const QSize &size, uint32_t format) +GbmGraphicsBuffer::GbmGraphicsBuffer(DmaBufAttributes attributes, gbm_bo *handle) : m_bo(handle) - , m_dmabufAttributes(dmaBufAttributesForBo(handle)) - , m_size(size) - , m_hasAlphaChannel(alphaChannelFromDrmFormat(format)) + , m_dmabufAttributes(std::move(attributes)) + , m_size(m_dmabufAttributes.width, m_dmabufAttributes.height) + , m_hasAlphaChannel(alphaChannelFromDrmFormat(m_dmabufAttributes.format)) { } diff --git a/src/core/gbmgraphicsbufferallocator.h b/src/core/gbmgraphicsbufferallocator.h index 0f67cad190..1f191a1f87 100644 --- a/src/core/gbmgraphicsbufferallocator.h +++ b/src/core/gbmgraphicsbufferallocator.h @@ -20,7 +20,7 @@ class KWIN_EXPORT GbmGraphicsBuffer : public GraphicsBuffer Q_OBJECT public: - GbmGraphicsBuffer(gbm_bo *handle, const QSize &size, uint32_t format); + GbmGraphicsBuffer(DmaBufAttributes attributes, gbm_bo *handle); ~GbmGraphicsBuffer() override; QSize size() const override;