From 151122aa2e0b8b233fad51dc0f53d64cd7f59e1b Mon Sep 17 00:00:00 2001
From: "William A. Kennington III" <wak@google.com>
Date: Tue, 15 May 2018 12:00:32 -0700
Subject: Add error handling to message parsing

Some of the messages we try and parse are not structured in the
format we expect when parsing. When error logging is enabled in
sdbusplus this causes the phosphor-fru-fault-manager to crash. Fix
this crashing by logging errors and ignoring the bad signals and
message responses.

Change-Id: Ie817beaae96961b33a343aabf1dbba9f31606d4b
Signed-off-by: William A. Kennington III <wak@google.com>
---
 fault-monitor/fru-fault-monitor.cpp | 54 ++++++++++++++++++++++++++++++++++---
 manager.cpp                         | 13 ++++++++-
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/fault-monitor/fru-fault-monitor.cpp b/fault-monitor/fru-fault-monitor.cpp
index 4a600f7..363cabc 100644
--- a/fault-monitor/fru-fault-monitor.cpp
+++ b/fault-monitor/fru-fault-monitor.cpp
@@ -1,4 +1,5 @@
 #include <phosphor-logging/elog.hpp>
+#include <sdbusplus/exception.hpp>
 #include "xyz/openbmc_project/Led/Fru/Monitor/error.hpp"
 #include "xyz/openbmc_project/Led/Mapper/error.hpp"
 #include "elog-errors.hpp"
@@ -72,7 +73,22 @@ std::string getService(sdbusplus::bus::bus& bus,
     }
 
     std::map<std::string, std::vector<std::string>> mapperResponse;
-    mapperResponseMsg.read(mapperResponse);
+    try
+    {
+        mapperResponseMsg.read(mapperResponse);
+    }
+    catch (const sdbusplus::exception::SdBusError& e)
+    {
+        log<level::ERR>("Failed to parse getService mapper response",
+                        entry("ERROR=%s", e.what()),
+                        entry("REPLY_SIG=%s", mapperResponseMsg.get_signature()));
+        using namespace xyz::openbmc_project::Led::Mapper;
+        elog<ObjectNotFoundErr>(
+            ObjectNotFoundError::METHOD_NAME("GetObject"),
+            ObjectNotFoundError::PATH(path.c_str()),
+            ObjectNotFoundError::INTERFACE(
+                OBJMGR_IFACE));
+    }
     if (mapperResponse.empty())
     {
         using namespace xyz::openbmc_project::Led::Mapper;
@@ -138,7 +154,17 @@ void Add::created(sdbusplus::message::message& msg)
     auto bus = msg.get_bus();
 
     LogEntryMsg logEntry;
-    msg.read(logEntry);
+    try
+    {
+        msg.read(logEntry);
+    }
+    catch (const sdbusplus::exception::SdBusError& e)
+    {
+        log<level::ERR>("Failed to parse created message",
+                        entry("ERROR=%s", e.what()),
+                        entry("REPLY_SIG=%s", msg.get_signature()));
+        return;
+    }
     std::string objectPath(std::move(logEntry.first));
 
     std::size_t found = objectPath.find(ELOG_ENTRY);
@@ -209,7 +235,17 @@ void Add::processExistingCallouts(sdbusplus::bus::bus& bus)
     }
 
     MapperResponseType mapperResponse;
-    mapperResponseMsg.read(mapperResponse);
+    try
+    {
+        mapperResponseMsg.read(mapperResponse);
+    }
+    catch (const sdbusplus::exception::SdBusError& e)
+    {
+        log<level::ERR>("Failed to parse existing callouts subtree message",
+                        entry("ERROR=%s", e.what()),
+                        entry("REPLY_SIG=%s", mapperResponseMsg.get_signature()));
+        return;
+    }
     if (mapperResponse.empty())
     {
         //No errors to process.
@@ -233,7 +269,17 @@ void Add::processExistingCallouts(sdbusplus::bus::bus& bus)
         }
 
         sdbusplus::message::variant<AssociationList> assoc;
-        reply.read(assoc);
+        try
+        {
+            reply.read(assoc);
+        }
+        catch (const sdbusplus::exception::SdBusError& e)
+        {
+            log<level::ERR>("Failed to parse existing callouts associations message",
+                            entry("ERROR=%s", e.what()),
+                            entry("REPLY_SIG=%s", reply.get_signature()));
+            continue;
+        }
         auto& assocs = assoc.get<AssociationList>();
         if (assocs.empty())
         {
diff --git a/manager.cpp b/manager.cpp
index 69d82b0..28e6815 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -2,6 +2,7 @@
 #include <string>
 #include <algorithm>
 #include <phosphor-logging/log.hpp>
+#include <sdbusplus/exception.hpp>
 #include <xyz/openbmc_project/Led/Physical/server.hpp>
 #include "manager.hpp"
 namespace phosphor
@@ -219,7 +220,17 @@ void Manager::populateObjectMap()
              std::vector<std::string>>> objectTree;
 
     // This is the dict of object paths - service names - interfaces
-    reply.read(objectTree);
+    try
+    {
+        reply.read(objectTree);
+    }
+    catch (const sdbusplus::exception::SdBusError& e)
+    {
+        log<level::ERR>("Failed to parse Physical LED service lookup",
+                        entry("ERROR=%s", e.what()),
+                        entry("REPLY_SIG=%s", reply.get_signature()));
+        return;
+    }
     if (objectTree.empty())
     {
         log<level::INFO>("Physical LED lookup did not return any services",
-- 
cgit v1.2.1