From 75e756295350a0ce37943edc76a7ca22076c1298 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 1 Apr 2021 13:09:37 +0300 Subject: [PATCH] platforms/drm: Fix handling of missing EDID drmModeGetPropertyBlob() may return null and we should handle that. In addition to that, m_conn is not initialized in DrmConnector so kwin will crash whenever the connector info is accessed. --- src/plugins/platforms/drm/drm_object.cpp | 3 ++ .../platforms/drm/drm_object_connector.cpp | 28 ++++++++----------- .../platforms/drm/drm_object_connector.h | 10 +++---- src/plugins/platforms/drm/drm_output.cpp | 1 - 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/plugins/platforms/drm/drm_object.cpp b/src/plugins/platforms/drm/drm_object.cpp index 0d339e1434..072967fa1d 100644 --- a/src/plugins/platforms/drm/drm_object.cpp +++ b/src/plugins/platforms/drm/drm_object.cpp @@ -51,6 +51,9 @@ bool DrmObject::initProps(const QVector &&vector, uint32_t o drmModePropertyBlobRes *blob = nullptr; if (prop->flags & DRM_MODE_PROP_BLOB) { blob = drmModeGetPropertyBlob(fd(), properties->prop_values[i]); + if (!blob) { + break; + } } qCDebug(KWIN_DRM, "Found property %s with value %lu", def.name.data(), properties->prop_values[i]); m_props[j] = new Property(prop.data(), properties->prop_values[i], def.enumNames, blob); diff --git a/src/plugins/platforms/drm/drm_object_connector.cpp b/src/plugins/platforms/drm/drm_object_connector.cpp index 73ec88c269..c703e8d890 100644 --- a/src/plugins/platforms/drm/drm_object_connector.cpp +++ b/src/plugins/platforms/drm/drm_object_connector.cpp @@ -9,7 +9,6 @@ #include "drm_object_connector.h" #include "drm_pointer.h" #include "logging.h" -#include "edid.h" #include // frameworks @@ -20,13 +19,10 @@ namespace KWin DrmConnector::DrmConnector(uint32_t connector_id, int fd) : DrmObject(connector_id, fd) + , m_conn(drmModeGetConnector(fd, connector_id)) { - DrmScopedPointer con(drmModeGetConnector(fd, connector_id)); - if (!con) { - return; - } - for (int i = 0; i < con->count_encoders; ++i) { - m_encoders << con->encoders[i]; + for (int i = 0; i < m_conn->count_encoders; ++i) { + m_encoders << m_conn->encoders[i]; } } @@ -55,8 +51,8 @@ bool DrmConnector::init() // parse edid if (auto edidProp = m_props[static_cast(PropertyIndex::Edid)]) { - m_edid.reset(new Edid(edidProp->blob()->data, edidProp->blob()->length)); - if (!m_edid->isValid()) { + m_edid = Edid(edidProp->blob()->data, edidProp->blob()->length); + if (!m_edid.isValid()) { qCWarning(KWIN_DRM, "Couldn't parse EDID for connector with id %d", id()); } deleteProp(PropertyIndex::Edid); @@ -65,21 +61,21 @@ bool DrmConnector::init() } // check the physical size - if (m_edid->physicalSize().isEmpty()) { + if (m_edid.physicalSize().isEmpty()) { m_physicalSize = QSize(m_conn->mmWidth, m_conn->mmHeight); } else { - m_physicalSize = m_edid->physicalSize(); + m_physicalSize = m_edid.physicalSize(); } // the size might be completely borked. E.g. Samsung SyncMaster 2494HS reports 160x90 while in truth it's 520x292 // as this information is used to calculate DPI info, it's going to result in everything being huge const QByteArray unknown = QByteArrayLiteral("unknown"); - KConfigGroup group = kwinApp()->config()->group("EdidOverwrite").group(m_edid->eisaId().isEmpty() ? unknown : m_edid->eisaId()) - .group(m_edid->monitorName().isEmpty() ? unknown : m_edid->monitorName()) - .group(m_edid->serialNumber().isEmpty() ? unknown : m_edid->serialNumber()); + KConfigGroup group = kwinApp()->config()->group("EdidOverwrite").group(m_edid.eisaId().isEmpty() ? unknown : m_edid.eisaId()) + .group(m_edid.monitorName().isEmpty() ? unknown : m_edid.monitorName()) + .group(m_edid.serialNumber().isEmpty() ? unknown : m_edid.serialNumber()); if (group.hasKey("PhysicalSize")) { const QSize overwriteSize = group.readEntry("PhysicalSize", m_physicalSize); - qCWarning(KWIN_DRM) << "Overwriting monitor physical size for" << m_edid->eisaId() << "/" << m_edid->monitorName() << "/" << m_edid->serialNumber() << " from " << m_physicalSize << "to " << overwriteSize; + qCWarning(KWIN_DRM) << "Overwriting monitor physical size for" << m_edid.eisaId() << "/" << m_edid.monitorName() << "/" << m_edid.serialNumber() << " from " << m_physicalSize << "to " << overwriteSize; m_physicalSize = overwriteSize; } @@ -125,7 +121,7 @@ QString DrmConnector::connectorName() const QString DrmConnector::modelName() const { - return connectorName() + m_edid->nameString(); + return connectorName() + m_edid.nameString(); } bool DrmConnector::isInternal() const diff --git a/src/plugins/platforms/drm/drm_object_connector.h b/src/plugins/platforms/drm/drm_object_connector.h index bb70d368e7..16440f6a37 100644 --- a/src/plugins/platforms/drm/drm_object_connector.h +++ b/src/plugins/platforms/drm/drm_object_connector.h @@ -12,12 +12,11 @@ #include #include "drm_object.h" +#include "edid.h" namespace KWin { -class Edid; - class DrmConnector : public DrmObject { public: @@ -53,8 +52,8 @@ public: return m_props[static_cast(PropertyIndex::Dpms)]; } - Edid *edid() const { - return m_edid.get(); + const Edid *edid() const { + return &m_edid; } QString connectorName() const; @@ -66,8 +65,7 @@ public: private: DrmScopedPointer m_conn; QVector m_encoders; - - QScopedPointer m_edid; + Edid m_edid; QSize m_physicalSize = QSize(-1, -1); }; diff --git a/src/plugins/platforms/drm/drm_output.cpp b/src/plugins/platforms/drm/drm_output.cpp index e69a9a0a02..612581d35e 100644 --- a/src/plugins/platforms/drm/drm_output.cpp +++ b/src/plugins/platforms/drm/drm_output.cpp @@ -10,7 +10,6 @@ #include "drm_backend.h" #include "drm_object_crtc.h" #include "drm_object_connector.h" -#include "edid.h" #include "composite.h" #include "cursor.h"