commit 25e20bd8cc614edb6bc3a335d6b4eb2e9df5c054
author Andrew Jeffery <andrew@aj.id.au> Tue Mar 15 22:26:04 2022 +1030
committer Andrew Jeffery <andrew@aj.id.au> Thu Mar 24 23:18:13 2022 +0000
tree 1cfaf27df3fee535c5bda2415eeba8cf337d7861
parent 8418975235cb792235e7cdc8fb4951be2167ced9

NVMeBasicContext: Reject bad I2C bus IDs

We can't possibly query such sensors, so ensure they can't exist.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I899979db57a84272be9b537d87e54756296695cf

include/NVMeContext.hpp

diff --git a/include/NVMeContext.hpp b/include/NVMeContext.hpp
index 27c1720..f8270f2 100644
--- a/include/NVMeContext.hpp
+++ b/include/NVMeContext.hpp
@@ -6,13 +6,20 @@
 #include <boost/asio/io_service.hpp>
 
 #include <memory>
+#include <stdexcept>
 
 class NVMeContext : public std::enable_shared_from_this<NVMeContext>
 {
   public:
     NVMeContext(boost::asio::io_service& io, int rootBus) :
         scanTimer(io), rootBus(rootBus)
-    {}
+    {
+        if (rootBus < 0)
+        {
+            throw std::invalid_argument(
+                "Invalid root bus: Bus ID must not be negative");
+        }
+    }
 
     virtual ~NVMeContext()
     {

src/NVMeBasicContext.cpp

diff --git a/src/NVMeBasicContext.cpp b/src/NVMeBasicContext.cpp
index e06a0a7..dacac26 100644
--- a/src/NVMeBasicContext.cpp
+++ b/src/NVMeBasicContext.cpp
@@ -260,17 +260,6 @@
         return;
     }
 
-    /* Ensure sensor query parameters are sensible */
-    if (sensor->bus < 0)
-    {
-        std::cerr << "Bus index cannot be negative: " << sensor->bus << "\n";
-
-        sensors.pop_front();
-        sensors.emplace_back(sensor);
-
-        return;
-    }
-
     auto command = encodeBasicQuery(sensor->bus, 0x6a, 0x00);
 
     /* Issue the request */

src/NVMeSensor.cpp

diff --git a/src/NVMeSensor.cpp b/src/NVMeSensor.cpp
index 89b21b0..d293737 100644
--- a/src/NVMeSensor.cpp
+++ b/src/NVMeSensor.cpp
@@ -33,6 +33,11 @@
            PowerState::on),
     bus(busNumber), objServer(objectServer)
 {
+    if (bus < 0)
+    {
+        throw std::invalid_argument("Invalid bus: Bus ID must not be negative");
+    }
+
     sensorInterface = objectServer.add_interface(
         "/xyz/openbmc_project/sensors/temperature/" + name,
         "xyz.openbmc_project.Sensor.Value");

src/NVMeSensorMain.cpp

diff --git a/src/NVMeSensorMain.cpp b/src/NVMeSensorMain.cpp
index f15f867..7e32815 100644
--- a/src/NVMeSensorMain.cpp
+++ b/src/NVMeSensorMain.cpp
@@ -152,13 +152,27 @@
                           << "\n";
             }
 
-            std::shared_ptr<NVMeSensor> sensorPtr =
-                std::make_shared<NVMeSensor>(
-                    objectServer, io, dbusConnection, *sensorName,
-                    std::move(sensorThresholds), interfacePath, *busNumber);
+            try
+            {
+                // May throw for an invalid rootBus
+                std::shared_ptr<NVMeContext> context =
+                    provideRootBusContext(io, nvmeDeviceMap, *rootBus);
 
-            provideRootBusContext(io, nvmeDeviceMap, *rootBus)
-                ->addSensor(sensorPtr);
+                // Construct the sensor after grabbing the context so we don't
+                // glitch D-Bus May throw for an invalid busNumber
+                std::shared_ptr<NVMeSensor> sensorPtr =
+                    std::make_shared<NVMeSensor>(
+                        objectServer, io, dbusConnection, *sensorName,
+                        std::move(sensorThresholds), interfacePath, *busNumber);
+
+                context->addSensor(sensorPtr);
+            }
+            catch (const std::invalid_argument& ex)
+            {
+                std::cerr << "Failed to add sensor for "
+                          << std::string(interfacePath) << ": " << ex.what()
+                          << "\n";
+            }
         }
     }
     for (const auto& [_, context] : nvmeDeviceMap)