From 06a0abff0124d0d0ce58f51621b27a735aa888f5 Mon Sep 17 00:00:00 2001
From: Brad Bishop <bradleyb@fuzziesquirrel.com>
Date: Mon, 26 Feb 2018 20:46:00 -0500
Subject: sensors: rework sensor assertion

Prior to this patch the sensor assertion logic would generate an invalid
DBus message or undefined behavior in several scenarios.

 - A bit is both asserted and desasserted:
A well behaving client won't do this but the existing logic would
generate an invalid property-set message with signature ssvv.  Rework
the logic such that no dbus traffic occurs.

 -  No bits are asserted:
Results in an invalid message with signature ss.
Rework such that no dbus traffic occurs.

 - Empty offset value map in configuration:
Results in an invalid message with signature s.
Rework such that no dbus traffic occurs.

 - Empty offset value map entry (either assert or deassert) in
   configuration:
Results in an invalid variant signature.
Rework such that no dbus traffic occurs.

 - The same bit is specified in the configuration for multiple
   properties or interfaces:
Can result in invalid messages with a wide variety of signatures.
Rework such that one message is sent for each property
being updated.

 - Invalid bit specified in value map entry
Results in undefined behavior calling bitset::test.
Rework such that entries in the value map with invalid bits
are ignored.

Tested: Verified the OperatingSystemStatus sensor can be set by a BIOS
Change-Id: I4df9472a8bdc9e44e98e1a963838da0912d10683
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
 sensordatahandler.cpp | 44 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/sensordatahandler.cpp b/sensordatahandler.cpp
index 7db8fe0..4d87341 100644
--- a/sensordatahandler.cpp
+++ b/sensordatahandler.cpp
@@ -256,33 +256,55 @@ ipmi_ret_t eventdata(const SetSensorReadingReq& cmdData,
 ipmi_ret_t assertion(const SetSensorReadingReq& cmdData,
                      const Info& sensorInfo)
 {
-    auto msg = makeDbusMsg(
-                   "org.freedesktop.DBus.Properties",
-                   sensorInfo.sensorPath,
-                   "Set",
-                   sensorInfo.sensorInterface);
-
     std::bitset<16> assertionSet(getAssertionSet(cmdData).first);
     std::bitset<16> deassertionSet(getAssertionSet(cmdData).second);
+    auto bothSet = assertionSet ^ deassertionSet;
 
     const auto& interface = sensorInfo.propertyInterfaces.begin();
-    msg.append(interface->first);
+
     for (const auto& property : interface->second)
     {
-        msg.append(property.first);
+        Value tmp{mapbox::util::no_init()};
         for (const auto& value : std::get<OffsetValueMap>(property.second))
         {
+            if (bothSet.size() <= value.first || !bothSet.test(value.first))
+            {
+                // A BIOS shouldn't do this but ignore if they do.
+                continue;
+            }
+
             if (assertionSet.test(value.first))
             {
-                msg.append(value.second.assert);
+                tmp = value.second.assert;
+                break;
             }
             if (deassertionSet.test(value.first))
             {
-                msg.append(value.second.deassert);
+                tmp = value.second.deassert;
+                break;
+            }
+        }
+
+        if (tmp.valid())
+        {
+            auto msg = makeDbusMsg(
+                   "org.freedesktop.DBus.Properties",
+                   sensorInfo.sensorPath,
+                   "Set",
+                   sensorInfo.sensorInterface);
+            msg.append(interface->first);
+            msg.append(property.first);
+            msg.append(tmp);
+
+            auto rc = updateToDbus(msg);
+            if (rc)
+            {
+                return rc;
             }
         }
     }
-    return updateToDbus(msg);
+
+    return IPMI_CC_OK;
 }
 
 }//namespace set
-- 
cgit v1.2.1