manager: add hard-coded read checks
The host-client cannot try to read back more than this number of bytes
without hitting a memory overflow in phosphor-host-ipmid. The method we
can call to dynamically receive that number isn't currently linking out
of the userlayer library.
Note: Once the configure_ac can find and provide the userlayer library
from phosphor-host-ipmid, we can remove the hard-coded 64 and 0xe.
Change-Id: I08f6bb59ce44279f76a494dccaa477ae75d532c4
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/manager.cpp b/manager.cpp
index 712d7db..639ca2d 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -270,8 +270,23 @@
return std::vector<uint8_t>();
}
+ /* TODO: Currently, configure_ac isn't finding libuserlayer, w.r.t the
+ * symbols I need.
+ */
+
+ /** The channel to use for now.
+ * TODO: We will receive this information through the IPMI message call.
+ */
+ // const int ipmiChannel = ipmi::currentChNum;
+ /** This information is transport specific.
+ * TODO: We need a way to know this dynamically.
+ * on BT, 4 bytes of header, and 1 reply code.
+ */
+ // uint32_t maxTransportSize = ipmi::getChannelMaxTransferSize(ipmiChannel);
+
/* Try reading from it. */
- return info->handler->read(session, offset, requestedSize);
+ return info->handler->read(session, offset,
+ std::min(maximumReadSize, requestedSize));
}
bool BlobManager::write(uint16_t session, uint32_t offset,
diff --git a/manager.hpp b/manager.hpp
index d7282e6..c0c4249 100644
--- a/manager.hpp
+++ b/manager.hpp
@@ -2,6 +2,7 @@
#include <blobs-ipmid/blobs.hpp>
#include <ctime>
+#include <host-ipmid/oemrouter.hpp>
#include <memory>
#include <string>
#include <unordered_map>
@@ -10,6 +11,16 @@
namespace blobs
{
+/* The maximum read size.
+ * NOTE: Once this can be dynamically determined, we'll switch to that method.
+ * Having this in a header allows it to used cleanly for now.
+ */
+const int crcSize = sizeof(uint16_t);
+const int btReplyHdrLen = 5;
+const int btTransportLength = 64;
+const uint32_t maximumReadSize =
+ btTransportLength - (btReplyHdrLen + oem::groupMagicSize + crcSize);
+
struct SessionInfo
{
SessionInfo() = default;
diff --git a/test/manager_read_unittest.cpp b/test/manager_read_unittest.cpp
index 1d40f5d..73e198b 100644
--- a/test/manager_read_unittest.cpp
+++ b/test/manager_read_unittest.cpp
@@ -60,7 +60,7 @@
uint16_t sess = 1;
uint32_t ofs = 0x54;
- uint32_t requested = 0x100;
+ uint32_t requested = 0x10;
uint16_t flags = OpenFlags::read;
std::string path = "/asdf/asdf";
std::vector<uint8_t> data = {0x12, 0x14, 0x15, 0x16};
@@ -75,4 +75,35 @@
EXPECT_EQ(data.size(), result.size());
EXPECT_EQ(result, data);
}
+
+TEST(ManagerReadTest, ReadTooManyBytesTruncates)
+{
+ // For now, the hard-coded maximum transfer size is 64 bytes on read
+ // commands, due to a hard-coded buffer in ipmid among other future
+ // challenges.
+
+ BlobManager mgr;
+ std::unique_ptr<BlobMock> m1 = std::make_unique<BlobMock>();
+ auto m1ptr = m1.get();
+ EXPECT_TRUE(mgr.registerHandler(std::move(m1)));
+
+ uint16_t sess = 1;
+ uint32_t ofs = 0x54;
+ uint32_t requested = 0x100;
+ uint16_t flags = OpenFlags::read;
+ std::string path = "/asdf/asdf";
+ std::vector<uint8_t> data = {0x12, 0x14, 0x15, 0x16};
+
+ EXPECT_CALL(*m1ptr, canHandleBlob(path)).WillOnce(Return(true));
+ EXPECT_CALL(*m1ptr, open(_, flags, path)).WillOnce(Return(true));
+ EXPECT_TRUE(mgr.open(flags, path, &sess));
+
+ EXPECT_CALL(*m1ptr, read(sess, ofs, maximumReadSize))
+ .WillOnce(Return(data));
+
+ std::vector<uint8_t> result = mgr.read(sess, ofs, requested);
+ EXPECT_EQ(data.size(), result.size());
+ EXPECT_EQ(result, data);
+}
+
} // namespace blobs