Revert "Add ipmi response buffer size check for all commands"
This reverts commit 88ca532a74375cc99bd979fbb037d07e5ee76cce.
Reason for revert: ipmi::getChannelMaxTransferSize is only using the
default channel and may fail when using a different channel.
Change-Id: Ic080de6f51c05f86807b81dfd3dd38a87fc2dbf5
Signed-off-by: Willy Tu <wltu@google.com>
diff --git a/ipmi.cpp b/ipmi.cpp
index d5954f2..07f6cd8 100644
--- a/ipmi.cpp
+++ b/ipmi.cpp
@@ -109,6 +109,8 @@
return IPMI_CC_INVALID_FIELD_REQUEST;
}
+ /* TODO(venture): Need to do a hard-code check against the maximum
+ * reply buffer size. */
reply->crc = 0;
/* Explicilty copies the NUL-terminator. */
std::memcpy(reply + 1, blobId.c_str(), blobId.length() + 1);
@@ -285,6 +287,9 @@
struct BmcBlobReadTx request;
std::memcpy(&request, reqBuf, sizeof(request));
+ /* TODO(venture): Verify requestedSize can fit in a returned IPMI packet.
+ */
+
std::vector<uint8_t> result =
mgr->read(request.sessionId, request.offset, request.requestedSize);
diff --git a/main.cpp b/main.cpp
index 63701c8..e90969b 100644
--- a/main.cpp
+++ b/main.cpp
@@ -24,13 +24,11 @@
#include <ipmid/api.h>
#include <cstdio>
-#include <functional>
#include <ipmid/iana.hpp>
#include <ipmid/oemopenbmc.hpp>
#include <ipmid/oemrouter.hpp>
#include <memory>
#include <phosphor-logging/log.hpp>
-#include <user_channel/channel_layer.hpp>
namespace blobs
{
@@ -46,13 +44,8 @@
"Registering OEM:[%#08X], Cmd:[%#04X] for Blob Commands\n",
oem::obmcOemNumber, oem::Cmd::blobTransferCmd);
- oemRouter->registerHandler(
- oem::obmcOemNumber, oem::Cmd::blobTransferCmd,
- std::bind_front(handleBlobCommand,
- // Get current IPMI channel and get the max transfer
- // size (assuming that it
- // does not change).
- ipmi::getChannelMaxTransferSize(ipmi::currentChNum)));
+ oemRouter->registerHandler(oem::obmcOemNumber, oem::Cmd::blobTransferCmd,
+ handleBlobCommand);
/* Install handlers. */
try
diff --git a/meson.build b/meson.build
index 21d113c..1c5808d 100644
--- a/meson.build
+++ b/meson.build
@@ -21,11 +21,7 @@
description: 'Phosphor Blob Transfer Interface',
version: meson.project_version())
-cpp = meson.get_compiler('cpp')
-
phosphor_logging_dep = dependency('phosphor-logging')
-ipmid_dep = dependency('libipmid')
-channellayer_dep = cpp.find_library('channellayer', required: true)
blob_manager_pre = declare_dependency(
dependencies: [
@@ -64,8 +60,7 @@
implicit_include_directories: false,
dependencies: [
blob_manager_dep,
- ipmid_dep,
- channellayer_dep,
+ dependency('libipmid'),
],
install: true,
install_dir: get_option('libdir') / 'ipmid-providers')
diff --git a/process.cpp b/process.cpp
index cdf73fa..2481fa2 100644
--- a/process.cpp
+++ b/process.cpp
@@ -110,7 +110,7 @@
ipmi_ret_t processBlobCommand(IpmiBlobHandler cmd, ManagerInterface* mgr,
const uint8_t* reqBuf, uint8_t* replyCmdBuf,
- size_t* dataLen, size_t maxSize)
+ size_t* dataLen)
{
ipmi_ret_t result = cmd(mgr, reqBuf, replyCmdBuf, dataLen);
if (result != IPMI_CC_OK)
@@ -134,12 +134,6 @@
return IPMI_CC_UNSPECIFIED_ERROR;
}
- /* Make sure the reply size fits the ipmi buffer */
- if (replyLength > maxSize)
- {
- return IPMI_CC_RESPONSE_ERROR;
- }
-
/* The command, whatever it was, replied, so let's set the CRC. */
std::vector<std::uint8_t> crcBuffer(replyCmdBuf + sizeof(uint16_t),
replyCmdBuf + replyLength);
@@ -150,7 +144,7 @@
return result;
}
-ipmi_ret_t handleBlobCommand(size_t maxSize, ipmi_cmd_t, const uint8_t* reqBuf,
+ipmi_ret_t handleBlobCommand(ipmi_cmd_t, const uint8_t* reqBuf,
uint8_t* replyCmdBuf, size_t* dataLen)
{
/* It's holding at least a sub-command. The OEN is trimmed from the bytes
@@ -172,7 +166,7 @@
}
return processBlobCommand(command, getBlobManager(), reqBuf, replyCmdBuf,
- dataLen, maxSize);
+ dataLen);
}
} // namespace blobs
diff --git a/process.hpp b/process.hpp
index 438f828..da0031c 100644
--- a/process.hpp
+++ b/process.hpp
@@ -36,18 +36,16 @@
* @param[in,out] replyCmdBuf - a pointer to the ipmi reply packet buffer.
* @param[in,out] dataLen - initially the request length, set to reply length
* on return.
- * @param[in,out] maxSize - Maximum ipmi reply size
* @return the ipmi command result.
*/
ipmi_ret_t processBlobCommand(IpmiBlobHandler cmd, ManagerInterface* mgr,
const uint8_t* reqBuf, uint8_t* replyCmdBuf,
- size_t* dataLen, size_t maxSize);
+ size_t* dataLen);
/**
* Given an IPMI command, request buffer, and reply buffer, validate the request
* and call processBlobCommand.
*/
-ipmi_ret_t handleBlobCommand(size_t maxSize, ipmi_cmd_t cmd,
- const uint8_t* reqBuf, uint8_t* replyCmdBuf,
- size_t* dataLen);
+ipmi_ret_t handleBlobCommand(ipmi_cmd_t cmd, const uint8_t* reqBuf,
+ uint8_t* replyCmdBuf, size_t* dataLen);
} // namespace blobs
diff --git a/test/process_unittest.cpp b/test/process_unittest.cpp
index dce8042..b72fe9a 100644
--- a/test/process_unittest.cpp
+++ b/test/process_unittest.cpp
@@ -200,8 +200,8 @@
dataLen = sizeof(request);
- EXPECT_EQ(IPMI_CC_INVALID, processBlobCommand(h, &manager, request, reply,
- &dataLen, MAX_IPMI_BUFFER));
+ EXPECT_EQ(IPMI_CC_INVALID,
+ processBlobCommand(h, &manager, request, reply, &dataLen));
}
TEST_F(ProcessBlobCommandTest, CommandReturnsOkWithNoPayload)
@@ -222,8 +222,8 @@
dataLen = sizeof(request);
- EXPECT_EQ(IPMI_CC_OK, processBlobCommand(h, &manager, request, reply,
- &dataLen, MAX_IPMI_BUFFER));
+ EXPECT_EQ(IPMI_CC_OK,
+ processBlobCommand(h, &manager, request, reply, &dataLen));
}
TEST_F(ProcessBlobCommandTest, CommandReturnsOkWithInvalidPayloadLength)
@@ -245,8 +245,7 @@
dataLen = sizeof(request);
EXPECT_EQ(IPMI_CC_UNSPECIFIED_ERROR,
- processBlobCommand(h, &manager, request, reply, &dataLen,
- MAX_IPMI_BUFFER));
+ processBlobCommand(h, &manager, request, reply, &dataLen));
}
TEST_F(ProcessBlobCommandTest, CommandReturnsOkWithValidPayloadLength)
@@ -271,36 +270,11 @@
EXPECT_CALL(crcMock, generateCrc(_)).WillOnce(Return(0x3412));
- EXPECT_EQ(IPMI_CC_OK, processBlobCommand(h, &manager, request, reply,
- &dataLen, MAX_IPMI_BUFFER));
+ EXPECT_EQ(IPMI_CC_OK,
+ processBlobCommand(h, &manager, request, reply, &dataLen));
EXPECT_EQ(dataLen, payloadLen);
uint8_t expectedBytes[3] = {0x12, 0x34, 0x56};
EXPECT_EQ(0, std::memcmp(expectedBytes, reply, sizeof(expectedBytes)));
}
-
-TEST_F(ProcessBlobCommandTest,
- CommandReturnsErrorWithReplyExceededMaxTransferSize)
-{
- // There is a minimum payload length of 3 bytes, this command returns a
- // payload of 3 bytes and the crc code is called to process the payload.
-
- StrictMock<ManagerMock> manager;
- size_t dataLen;
- uint8_t request[MAX_IPMI_BUFFER] = {0};
- uint8_t reply[MAX_IPMI_BUFFER] = {0};
- uint32_t payloadLen = sizeof(uint16_t) + sizeof(uint8_t);
-
- IpmiBlobHandler h = [payloadLen](ManagerInterface*, const uint8_t*,
- uint8_t* replyCmdBuf, size_t* dataLen) {
- (*dataLen) = payloadLen;
- replyCmdBuf[2] = 0x56;
- return IPMI_CC_OK;
- };
-
- dataLen = sizeof(request);
-
- EXPECT_EQ(IPMI_CC_RESPONSE_ERROR,
- processBlobCommand(h, &manager, request, reply, &dataLen, 0));
-}
} // namespace blobs