Ensure using correct string length when using transaction id journal data

journald does not guarantee that sd_journal_get() returns NULL terminated
strings. Specify the actual length of the string (instead of relying on
functions that look for NULL terminators to determine the length) to
compare the transaction id number, otherwise intermittent errors can occur.

Change-Id: I58c34f377866f002a09b07810065b4e064f216b9
Closes: openbmc/openbmc#1217
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>

1 files changed, 16 insertions, 4 deletions

diff --git a/log_manager.cpp b/log_manager.cpp
index d8f233c..3401e82 100644
--- a/log_manager.cpp
+++ b/log_manager.cpp
@@ -23,7 +23,10 @@ namespace logging
 void Manager::commit(uint64_t transactionId, std::string errMsg)
 {
     constexpr const auto transactionIdVar = "TRANSACTION_ID";
+    // Length of 'TRANSACTION_ID' string.
     constexpr const auto transactionIdVarSize = strlen(transactionIdVar);
+    // Length of 'TRANSACTION_ID=' string.
+    constexpr const auto transactionIdVarOffset = transactionIdVarSize + 1;
 
     sd_journal *j = nullptr;
     int rc = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY);
@@ -65,10 +68,19 @@ void Manager::commit(uint64_t transactionId, std::string errMsg)
             continue;
         }
 
-        // The metadata field result will be TRANSACTION_ID=1234. Skip the
-        // TRANSACTION_ID piece and (=) sign to get the id number to compare.
-        if (strcmp((data + transactionIdVarSize + 1),
-                    transactionIdStr.c_str()) != 0)
+        // journald does not guarantee that sd_journal_get_data() returns NULL
+        // terminated strings, so need to specify the size to use to compare,
+        // use the returned length instead of anything that relies on NULL
+        // terminators like strlen().
+        // The data variable is in the form of 'TRANSACTION_ID=1234'. Remove
+        // the TRANSACTION_ID characters plus the (=) sign to do the comparison.
+        // 'data + transactionIdVarOffset' will be in the form of '1234'.
+        // 'length - transactionIdVarOffset' will be the length of '1234'.
+        if ((length <= (transactionIdVarOffset)) ||
+            (transactionIdStr.compare(0,
+                                      transactionIdStr.size(),
+                                      data + transactionIdVarOffset,
+                                      length - transactionIdVarOffset) != 0))
         {
             // The value of the TRANSACTION_ID metadata is not the requested
             // transaction id number.