commit 27c87d922b94567b05ba4c60ba67615c1568b599
author Adriana Kobylak <anoo@us.ibm.com> Mon Mar 06 12:45:09 2017 -0600
committer Adriana Kobylak <anoo@us.ibm.com> Tue Mar 07 22:08:39 2017 -0600
tree ff0e99140524ad1539ba6721e3548229f59aab4b
parent 6721899ba8f85d201ba2f7f394f5b5027aedfd8b

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>

log_manager.cpp

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 @@
 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 @@
             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.