[arista] Limit eeprom reads to 512 bytes

CMakeLists.txt

@@ -1047,6 +1047,7 @@ add_dependencies(fboss_platform_services
   platform_manager_handler_test
   weutil_crc16_ccitt_test
   weutil_fboss_eeprom_interface_test
+  weutil_parser_utils_test
   sensor_service_sw_test
   sensor_service_utils_test
   rackmon_test

cmake/PlatformWeutilTest.cmake

@@ -28,3 +28,16 @@ target_link_libraries(weutil_fboss_eeprom_interface_test
 )
 
 gtest_discover_tests(weutil_fboss_eeprom_interface_test)
+
+add_executable(weutil_parser_utils_test
+  fboss/platform/weutil/test/ParserUtilsTest.cpp
+)
+
+target_link_libraries(weutil_parser_utils_test
+  weutil_fboss_eeprom_interface
+  Folly::folly
+  ${GTEST}
+  ${LIBGMOCK_LIBRARIES}
+)
+
+gtest_discover_tests(weutil_parser_utils_test)

fboss/platform/platform_manager/PlatformExplorer.cpp

@@ -19,6 +19,7 @@
 #include "fboss/platform/platform_manager/Utils.h"
 #include "fboss/platform/weutil/FbossEepromInterface.h"
 #include "fboss/platform/weutil/IoctlSmbusEepromReader.h"
+#include "fboss/platform/weutil/ParserUtils.h"
 
 namespace facebook::fboss::platform::platform_manager {
 namespace {
@@ -335,6 +336,7 @@ std::optional<std::string> PlatformExplorer::getPmUnitNameFromSlot(
     directly with ioctl and written to the /run/devmap file. See:
     https://github.com/facebookexternal/fboss.bsp.arista/pull/31/files
     */
+    uint16_t eepromOffset = *idpromConfig.offset();
     if ((platformConfig_.platformName().value() == "MERU800BFA" ||
          platformConfig_.platformName().value() == "MERU800BIA" ||
          platformConfig_.platformName().value() == "ICECUBE800BANW" ||
@@ -348,9 +350,12 @@ std::optional<std::string> PlatformExplorer::getPmUnitNameFromSlot(
         IoctlSmbusEepromReader::readEeprom(
             eepromDir,
             eepromName,
-            0,
+            *idpromConfig.offset(),
             std::stoi(*idpromConfig.address(), nullptr, 16),
-            dataStore_.getI2cBusNum(slotPath, *idpromConfig.busName()));
+            dataStore_.getI2cBusNum(slotPath, *idpromConfig.busName()),
+            kMaxEepromDataRegionSize);
+        // The parsed file contains data starting at offset 0
+        eepromOffset = 0;
       } catch (const std::exception& e) {
         auto errMsg = fmt::format(
             "Could not read MERU_SCM_EEPROM for {}: {}",
@@ -373,8 +378,7 @@ std::optional<std::string> PlatformExplorer::getPmUnitNameFromSlot(
     try {
       auto idpromDevicePath = Utils().createDevicePath(slotPath, "IDPROM");
       dataStore_.updateEepromContents(
-          idpromDevicePath,
-          FbossEepromInterface(eepromPath, *idpromConfig.offset()));
+          idpromDevicePath, FbossEepromInterface(eepromPath, eepromOffset));
       const auto& eepromContents =
           dataStore_.getEepromContents(idpromDevicePath);
       if (idpromDevicePath == *platformConfig_.chassisEepromDevicePath()) {

fboss/platform/weutil/ConfigUtils.cpp

@@ -160,7 +160,7 @@ std::unordered_map<std::string, FruEeprom> ConfigUtils::getFruEepromList() {
     std::string eepromName = "SCM";
     FruEeprom fruEeprom;
     fruEeprom.path = "/run/devmap/eeproms/MERU_SCM_EEPROM";
-    fruEeprom.offset = getEepromOffset(config_, eepromName);
+    fruEeprom.offset = 0;
     fruEepromList[eepromName] = fruEeprom;
   } else if (config_.platformName().value() == "DARWIN") {
     // Darwin Platform special case that doesn't have a chassis EEPROM path

fboss/platform/weutil/ParserUtils.cpp

@@ -1,7 +1,6 @@
 // (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
 #include <fmt/format.h>
 #include <fstream>
-#include <iostream>
 #include <sstream>
 #include <stdexcept>
 
@@ -53,42 +52,27 @@ std::vector<uint8_t> ParserUtils::loadEeprom(
     const std::string& eeprom,
     int offset) {
   // Declare buffer, and fill it up with 0s
-  long fileSize = 0;
-  long bytesToRead = 0;
   std::ifstream file(eeprom, std::ios::binary);
-  std::vector<uint8_t> result;
-  // First, detect EEPROM size
-  try {
-    file.seekg(0, std::ios::end);
-    fileSize = file.tellg();
-    if (fileSize < 0) {
-      throw std::runtime_error(
-          fmt::format("EEPROM {} does not exist, or is empty!", eeprom));
-    }
+  std::vector<uint8_t> result(kMaxEepromDataRegionSize);
 
-    // bytesToRead cannot be bigger than the remaining bytes of the file from
-    // the offset. That is, we cannot read beyond the end of the file.
-    // If the remaining bytes are smaller than max, then we only read up to
-    // the end of the file.
-    bytesToRead = fileSize - offset;
-    if (bytesToRead < 0) {
-      throw std::runtime_error("Offset greater than file size");
-    }
-    result.resize(bytesToRead);
-  } catch (std::exception& ex) {
-    std::cout << "Failed to detect EEPROM size (" << eeprom
-              << "): " << ex.what() << std::endl;
-    throw std::runtime_error("Unable to detect EEPROM size.");
+  // First, validate file can be opened
+  if (!file) {
+    throw std::runtime_error(fmt::format("Failed to open EEPROM {}.", eeprom));
   }
 
   // Now, read the eeprom
   try {
     file.seekg(offset, std::ios::beg);
-    file.read((char*)result.data(), bytesToRead);
+    file.read((char*)result.data(), kMaxEepromDataRegionSize);
+    if (file.gcount() == 0) {
+      throw std::runtime_error("Offset greater than file size");
+    }
     file.close();
   } catch (std::exception& ex) {
-    std::cout << "Failed to read EEPROM contents " << ex.what() << std::endl;
+    throw std::runtime_error(
+        fmt::format("Failed to read EEPROM contents: {}", ex.what()));
   }
+
   return result;
 }
 

fboss/platform/weutil/ParserUtils.h

@@ -7,6 +7,9 @@
 
 namespace facebook::fboss::platform {
 
+// Maximum size of the EEPROM data region to read.
+constexpr long kMaxEepromDataRegionSize = 512;
+
 class ParserUtils {
  public:
   static std::vector<uint8_t> loadEeprom(const std::string& eeprom, int offset);

fboss/platform/weutil/test/BUCK

@@ -56,3 +56,15 @@ cpp_unittest(
         "//thrift/lib/cpp2/protocol:protocol",
     ],
 )
+
+cpp_unittest(
+    name = "parser_utils_test",
+    srcs = [
+        "ParserUtilsTest.cpp",
+    ],
+    deps = [
+        "//fboss/platform/weutil:fboss_eeprom_lib",
+        "//folly:file_util",
+        "//folly/testing:test_util",
+    ],
+)

fboss/platform/weutil/test/ParserUtilsTest.cpp

@@ -0,0 +1,33 @@
+// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
+
+#include <gtest/gtest.h>
+
+#include <folly/FileUtil.h>
+#include <folly/testing/TestUtil.h>
+
+#include "fboss/platform/weutil/ParserUtils.h"
+
+namespace facebook::fboss::platform {
+
+class ParserUtilsTest : public ::testing::Test {
+ protected:
+  std::string writeTempFile(const std::vector<uint8_t>& data) {
+    std::string path = tmpDir_.path().string() + "/eeprom";
+    folly::writeFile(data, path.c_str());
+    return path;
+  }
+
+ private:
+  folly::test::TemporaryDirectory tmpDir_;
+};
+
+// Verify loadEeprom() reads at most 512 bytes regardless of file size.
+TEST_F(ParserUtilsTest, LargerThan512BytesReturns512Bytes) {
+  std::vector<uint8_t> data(600, 0xAB);
+  auto path = writeTempFile(data);
+  auto result = ParserUtils::loadEeprom(path, 0);
+  EXPECT_EQ(result.size(), kMaxEepromDataRegionSize);
+  EXPECT_EQ(result, std::vector<uint8_t>(kMaxEepromDataRegionSize, 0xAB));
+}
+
+} // namespace facebook::fboss::platform