From d55fa39ebf4b758f191700539ba06c09305189ee Mon Sep 17 00:00:00 2001
From: Aleix Pol Gonzalez <aleix.pol_gonzalez@mercedes-benz.com>
Date: Fri, 3 May 2024 18:47:38 +0200
Subject: [PATCH] backends/drm: Add a check for ranged properties

Include a warning if we are ever setting an invalid value, this can save
some of us quite some time in the future, since failing to fall in the
range makes the whole pipeline become rejected.
---
 src/backends/drm/drm_commit.cpp   |  1 +
 src/backends/drm/drm_property.cpp | 26 +++++++++++++++++---------
 src/backends/drm/drm_property.h   | 15 +++++++++------
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/backends/drm/drm_commit.cpp b/src/backends/drm/drm_commit.cpp
index e80765d714..d270223a20 100644
--- a/src/backends/drm/drm_commit.cpp
+++ b/src/backends/drm/drm_commit.cpp
@@ -44,6 +44,7 @@ DrmAtomicCommit::DrmAtomicCommit(const QList<DrmPipeline *> &pipelines)
 
 void DrmAtomicCommit::addProperty(const DrmProperty &prop, uint64_t value)
 {
+    prop.checkValueInRange(value);
     m_properties[prop.drmObject()->id()][prop.propId()] = value;
 }
 
diff --git a/src/backends/drm/drm_property.cpp b/src/backends/drm/drm_property.cpp
index f9ae9baf1b..bc1e5a9279 100644
--- a/src/backends/drm/drm_property.cpp
+++ b/src/backends/drm/drm_property.cpp
@@ -42,9 +42,7 @@ void DrmProperty::update(DrmPropertyList &propertyList)
         const auto &[prop, value] = *opt;
         m_propId = prop->prop_id;
         m_current = value;
-        m_immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
-        m_isBlob = prop->flags & DRM_MODE_PROP_BLOB;
-        m_isBitmask = prop->flags & DRM_MODE_PROP_BITMASK;
+        m_flags = prop->flags;
         if (prop->flags & DRM_MODE_PROP_RANGE) {
             Q_ASSERT(prop->count_values > 1);
             m_minValue = prop->values[0];
@@ -58,7 +56,7 @@ void DrmProperty::update(DrmPropertyList &propertyList)
                 struct drm_mode_property_enum *en = &prop->enums[i];
                 int j = m_enumNames.indexOf(QByteArray(en->name));
                 if (j >= 0) {
-                    if (m_isBitmask) {
+                    if (m_flags & DRM_MODE_PROP_BITMASK) {
                         m_enumToPropertyMap[1 << j] = 1 << en->value;
                         m_propertyToEnumMap[1 << en->value] = 1 << j;
                     } else {
@@ -68,7 +66,7 @@ void DrmProperty::update(DrmPropertyList &propertyList)
                 }
             }
         }
-        if (m_immutable && m_isBlob) {
+        if ((m_flags & DRM_MODE_PROP_IMMUTABLE) && (m_flags & DRM_MODE_PROP_BLOB)) {
             if (m_current != 0) {
                 m_immutableBlob.reset(drmModeGetPropertyBlob(m_obj->gpu()->fd(), m_current));
                 if (m_immutableBlob && (!m_immutableBlob->data || !m_immutableBlob->length)) {
@@ -108,12 +106,17 @@ const QByteArray &DrmProperty::name() const
 
 bool DrmProperty::isImmutable() const
 {
-    return m_immutable;
+    return m_flags & DRM_MODE_PROP_IMMUTABLE;
 }
 
 bool DrmProperty::isBitmask() const
 {
-    return m_isBitmask;
+    return m_flags & DRM_MODE_PROP_BITMASK;
+}
+
+uint64_t DrmProperty::maxValue() const
+{
+    return m_maxValue;
 }
 
 uint64_t DrmProperty::minValue() const
@@ -121,9 +124,14 @@ uint64_t DrmProperty::minValue() const
     return m_minValue;
 }
 
-uint64_t DrmProperty::maxValue() const
+void DrmProperty::checkValueInRange(uint64_t value) const
 {
-    return m_maxValue;
+    if (Q_UNLIKELY(m_flags & DRM_MODE_PROP_RANGE && (value > m_maxValue || value < m_minValue))) {
+        qCWarning(KWIN_DRM) << "Range property value out of bounds." << m_propName << " value:" << value << "min:" << m_minValue << "max:" << m_maxValue;
+    }
+    if (Q_UNLIKELY(m_flags & DRM_MODE_PROP_SIGNED_RANGE && (int64_t(value) > int64_t(m_maxValue) || int64_t(value) < int64_t(m_minValue)))) {
+        qCWarning(KWIN_DRM) << "Signed range property value out of bounds." << m_propName << " value:" << int64_t(value) << "min:" << int64_t(m_minValue) << "max:" << int64_t(m_maxValue);
+    }
 }
 
 drmModePropertyBlobRes *DrmProperty::immutableBlob() const
diff --git a/src/backends/drm/drm_property.h b/src/backends/drm/drm_property.h
index 72b3b4aaaa..d618f62f7e 100644
--- a/src/backends/drm/drm_property.h
+++ b/src/backends/drm/drm_property.h
@@ -42,6 +42,11 @@ public:
     uint64_t maxValue() const;
     bool isValid() const;
 
+    /**
+     * Prints a warning and returns false if @p value is unacceptable for the property
+     */
+    void checkValueInRange(uint64_t value) const;
+
     void update(DrmPropertyList &propertyList);
     bool setPropertyLegacy(uint64_t value);
 
@@ -60,9 +65,7 @@ protected:
 
     QMap<uint64_t, uint64_t> m_enumToPropertyMap;
     QMap<uint64_t, uint64_t> m_propertyToEnumMap;
-    bool m_immutable = false;
-    bool m_isBlob = false;
-    bool m_isBitmask = false;
+    uint32_t m_flags;
 };
 
 template<typename Enum>
@@ -82,7 +85,7 @@ public:
     bool hasEnum(Enum value) const
     {
         const uint64_t integerValue = static_cast<uint64_t>(value);
-        if (m_isBitmask) {
+        if (isBitmask()) {
             for (uint64_t mask = 1; integerValue >= mask && mask != 0; mask <<= 1) {
                 if ((integerValue & mask) && !m_enumToPropertyMap.contains(mask)) {
                     return false;
@@ -96,7 +99,7 @@ public:
 
     Enum enumForValue(uint64_t value) const
     {
-        if (m_isBitmask) {
+        if (isBitmask()) {
             uint64_t ret = 0;
             for (uint64_t mask = 1; value >= mask && mask != 0; mask <<= 1) {
                 if (value & mask) {
@@ -112,7 +115,7 @@ public:
     uint64_t valueForEnum(Enum enumValue) const
     {
         const uint64_t integer = static_cast<uint64_t>(enumValue);
-        if (m_isBitmask) {
+        if (isBitmask()) {
             uint64_t set = 0;
             for (uint64_t mask = 1; integer >= mask && mask != 0; mask <<= 1) {
                 if (integer & mask) {