buffer: Update to follow the design doc
queueSize and ueRegionSize should not change at runtime. Make it so that
it throws if these constants fail any time we try to use them as it
means that the buffer was overwritten (possibly by a bad actor).
Update the default queueSize and ueRegionSize to match our test
variables.
Signed-off-by: Brandon Kim <brandonkim@google.com>
Change-Id: I2c906d26016ce6eb19e953ae432abd5482b6ccf8
diff --git a/include/buffer.hpp b/include/buffer.hpp
index 29f3e2f..37bcb9f 100644
--- a/include/buffer.hpp
+++ b/include/buffer.hpp
@@ -161,6 +161,12 @@
* * @return Queue size - UE region size - Queue header size
*/
virtual size_t getMaxOffset() = 0;
+
+ /** @brief The Error log queue starts after the UE region, which is where
+ * the read and write pointers are offset from relatively
+ * @return relative offset for read and write pointers
+ */
+ virtual size_t getQueueOffset() = 0;
};
/**
@@ -186,13 +192,9 @@
EntryPair readEntry() override;
std::vector<EntryPair> readErrorLogs() override;
size_t getMaxOffset() override;
+ size_t getQueueOffset() override;
private:
- /** @brief The Error log queue starts after the UE region, which is where
- * the read and write pointers are offset from relatively
- * @return relative offset for read and write pointers
- */
- size_t getQueueOffset();
/** @brief Calculate the checksum by XOR each bytes in the span
* @param[in] entry - Span to calculate the checksum on
* @return calculated checksum
diff --git a/meson_options.txt b/meson_options.txt
index ea8eb6b..ef68219 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -10,8 +10,8 @@
# Circular Buffer header constants
option('bmc-interface-version', type: 'integer', value: 0, description: 'BMC interface version to easily see compatibility')
-option('queue-region-size', type: 'integer', value: 0, description: 'Normal error queue region size')
-option('ue-region-size', type: 'integer', value: 0, description: 'Uncorrectable error region size')
+option('queue-region-size', type: 'integer', value: 512, description: 'Normal error queue region size')
+option('ue-region-size', type: 'integer', value: 80, description: 'Uncorrectable error region size')
# Magic Number Array Constants
option('magic-number-byte1', type: 'integer', value: 1, description: 'Magic Number array[0] for validity, consists of 4 * uint32_t')
option('magic-number-byte2', type: 'integer', value: 2, description: 'Magic Number array[1] for validity, consists of 4 * uint32_t')
diff --git a/src/buffer.cpp b/src/buffer.cpp
index b38862e..895c65a 100644
--- a/src/buffer.cpp
+++ b/src/buffer.cpp
@@ -1,3 +1,5 @@
+#include "config.h"
+
#include "buffer.hpp"
#include "pci_handler.hpp"
@@ -140,12 +142,6 @@
cachedBufferHeader.bmcFlags = littleNewBmcFlag;
}
-size_t BufferImpl::getQueueOffset()
-{
- return sizeof(struct CircularBufferHeader) +
- boost::endian::little_to_native(cachedBufferHeader.ueRegionSize);
-}
-
std::vector<uint8_t> BufferImpl::wraparoundRead(const uint32_t relativeOffset,
const uint32_t length)
{
@@ -328,7 +324,37 @@
size_t ueRegionSize =
boost::endian::little_to_native(cachedBufferHeader.ueRegionSize);
+ if (queueSize != QUEUE_REGION_SIZE)
+ {
+ throw std::runtime_error(fmt::format(
+ "[{}] runtime queueSize '{}' did not match compile-time queueSize "
+ "'{}'. This indicates that the buffer was corrupted",
+ __FUNCTION__, queueSize, QUEUE_REGION_SIZE));
+ }
+ if (ueRegionSize != UE_REGION_SIZE)
+ {
+ throw std::runtime_error(fmt::format(
+ "[{}] runtime ueRegionSize '{}' did not match compile-time "
+ "ueRegionSize '{}'. This indicates that the buffer was corrupted",
+ __FUNCTION__, ueRegionSize, UE_REGION_SIZE));
+ }
+
return queueSize - ueRegionSize - sizeof(struct CircularBufferHeader);
}
+size_t BufferImpl::getQueueOffset()
+{
+ size_t ueRegionSize =
+ boost::endian::little_to_native(cachedBufferHeader.ueRegionSize);
+
+ if (ueRegionSize != UE_REGION_SIZE)
+ {
+ throw std::runtime_error(fmt::format(
+ "[{}] runtime ueRegionSize '{}' did not match compile-time "
+ "ueRegionSize '{}'. This indicates that the buffer was corrupted",
+ __FUNCTION__, ueRegionSize, UE_REGION_SIZE));
+ }
+ return sizeof(struct CircularBufferHeader) + ueRegionSize;
+}
+
} // namespace bios_bmc_smm_error_logger
diff --git a/test/buffer_test.cpp b/test/buffer_test.cpp
index 2856e8c..d92bf06 100644
--- a/test/buffer_test.cpp
+++ b/test/buffer_test.cpp
@@ -240,6 +240,90 @@
static_cast<uint32_t>(BmcFlags::ready));
}
+TEST_F(BufferTest, GetMaxOffsetQueueSizeFail)
+{
+ InSequence s;
+ static constexpr size_t wrongQueueSize = testQueueSize - 1;
+ EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
+ .WillOnce(Return(testRegionSize));
+ const std::vector<uint8_t> emptyArray(wrongQueueSize, 0);
+ EXPECT_CALL(*dataInterfaceMockPtr, write(0, ElementsAreArray(emptyArray)))
+ .WillOnce(Return(wrongQueueSize));
+
+ EXPECT_CALL(*dataInterfaceMockPtr, write(0, _))
+ .WillOnce(Return(bufferHeaderSize));
+ EXPECT_NO_THROW(bufferImpl->initialize(testBmcInterfaceVersion,
+ wrongQueueSize, testUeRegionSize,
+ testMagicNumber));
+ EXPECT_THROW(
+ try {
+ bufferImpl->getMaxOffset();
+ } catch (const std::runtime_error& e) {
+ EXPECT_STREQ(e.what(),
+ "[getMaxOffset] runtime queueSize '511' did not match "
+ "compile-time queueSize '512'. This indicates that the"
+ " buffer was corrupted");
+ throw;
+ },
+ std::runtime_error);
+}
+
+TEST_F(BufferTest, GetMaxOffsetUeRegionSizeFail)
+{
+ InSequence s;
+ EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
+ .WillOnce(Return(testRegionSize));
+ const std::vector<uint8_t> emptyArray(testQueueSize, 0);
+ EXPECT_CALL(*dataInterfaceMockPtr, write(0, ElementsAreArray(emptyArray)))
+ .WillOnce(Return(testQueueSize));
+
+ EXPECT_CALL(*dataInterfaceMockPtr, write(0, _))
+ .WillOnce(Return(bufferHeaderSize));
+ EXPECT_NO_THROW(bufferImpl->initialize(testBmcInterfaceVersion,
+ testQueueSize, testUeRegionSize + 1,
+ testMagicNumber));
+ EXPECT_THROW(
+ try {
+ bufferImpl->getMaxOffset();
+ } catch (const std::runtime_error& e) {
+ EXPECT_STREQ(
+ e.what(),
+ "[getMaxOffset] runtime ueRegionSize '81' did not match "
+ "compile-time ueRegionSize '80'. This indicates that the"
+ " buffer was corrupted");
+ throw;
+ },
+ std::runtime_error);
+}
+
+TEST_F(BufferTest, GetOffsetUeRegionSizeFail)
+{
+ InSequence s;
+ EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
+ .WillOnce(Return(testRegionSize));
+ const std::vector<uint8_t> emptyArray(testQueueSize, 0);
+ EXPECT_CALL(*dataInterfaceMockPtr, write(0, ElementsAreArray(emptyArray)))
+ .WillOnce(Return(testQueueSize));
+
+ EXPECT_CALL(*dataInterfaceMockPtr, write(0, _))
+ .WillOnce(Return(bufferHeaderSize));
+ EXPECT_NO_THROW(bufferImpl->initialize(testBmcInterfaceVersion,
+ testQueueSize, testUeRegionSize - 1,
+ testMagicNumber));
+ EXPECT_THROW(
+ try {
+ bufferImpl->getQueueOffset();
+ } catch (const std::runtime_error& e) {
+ EXPECT_STREQ(
+ e.what(),
+ "[getQueueOffset] runtime ueRegionSize '79' did not match "
+ "compile-time ueRegionSize '80'. This indicates that the"
+ " buffer was corrupted");
+ throw;
+ },
+ std::runtime_error);
+}
+
class BufferWraparoundReadTest : public BufferTest
{
protected:
@@ -274,11 +358,17 @@
reinterpret_cast<uint8_t*>(&testInitializationHeader);
};
-TEST_F(BufferWraparoundReadTest, GetMaxOffsetTest)
+TEST_F(BufferWraparoundReadTest, GetMaxOffsetPassTest)
{
EXPECT_EQ(bufferImpl->getMaxOffset(), testMaxOffset);
}
+TEST_F(BufferWraparoundReadTest, GetQueueOffsetPassTest)
+{
+ EXPECT_EQ(bufferImpl->getQueueOffset(),
+ bufferHeaderSize + testUeRegionSize);
+}
+
TEST_F(BufferWraparoundReadTest, ParamsTooBigFail)
{
InSequence s;