fru-device: Own the bus list copy to prevent crash _i2cBuses was a reference to the caller's local bus list. This list is gone as soon as the caller returns, but the retry runs almost 5s later on a timer, so it ends up using memory that no longer exists and cause fru-device crashes. Fix this by changing the reference into an owned copy. Tested: Before: sometimes fru-device will core-dumps within 15 loops ac cycle: ``` Jun 08 03:37:51 bmc systemd[1]: xyz.openbmc_project.FruDevice.service: Main process exited, code=dumped, status=11/SEGV Jun 08 03:37:51 bmc systemd[1]: xyz.openbmc_project.FruDevice.service: Failed with result 'core-dump'. Jun 08 03:37:51 bmc systemd[1]: xyz.openbmc_project.FruDevice.service: Consumed 1.551s CPU time. Jun 08 03:37:56 bmc systemd[1]: xyz.openbmc_project.FruDevice.service: Scheduled restart job, restart counter is at 1. ``` After: fru-device will not core-dumps within 235 loops ac cycle. Google-Bug-Id: 518601929 Google-Bug-Id: 515874892 Change-Id: Iddd049ed4247be4abb9043c6af589cde6e80b581 Signed-off-by: Jeff Lin <jefflin2@quanta.corp-partner.google.com>
diff --git a/recipes-phosphor/configuration/entity-manager/0003-fru-device-Add-retries-on-locked-up-buses.patch b/recipes-phosphor/configuration/entity-manager/0003-fru-device-Add-retries-on-locked-up-buses.patch index cf4f5d2..7252d2a 100644 --- a/recipes-phosphor/configuration/entity-manager/0003-fru-device-Add-retries-on-locked-up-buses.patch +++ b/recipes-phosphor/configuration/entity-manager/0003-fru-device-Add-retries-on-locked-up-buses.patch
@@ -1,7 +1,7 @@ -From a892b143df6961c954df712a71333522d902576b Mon Sep 17 00:00:00 2001 +From a7b8abe5e2347a181e6d974e426e9b093134183c Mon Sep 17 00:00:00 2001 From: Willy Tu <wltu@google.com> Date: Thu, 29 Jan 2026 23:39:19 +0000 -Subject: [PATCH 3/5] fru-device: Add retries on locked up buses +Subject: [PATCH] fru-device: Add retries on locked up buses Try to rescan the buses that timed out to make sure we don't accidentally drop the i2c bus when it was busy during rescan. @@ -25,16 +25,16 @@ Change-Id: I3ebe5bef9be3bf74292a34ee8b1c0881f5cb0a94 Signed-off-by: Willy Tu <wltu@google.com> --- - meson_options.txt | 12 +++++++++++ - src/fru_device.cpp | 53 +++++++++++++++++++++++++++++++++++++++------- - src/meson.build | 12 +++++++++++ - 3 files changed, 69 insertions(+), 8 deletions(-) + meson_options.txt | 12 ++++++++ + src/fru_device.cpp | 68 ++++++++++++++++++++++++++++++++++++---------- + src/meson.build | 12 ++++++++ + 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/meson_options.txt b/meson_options.txt -index c97b7f8..7cf9336 100644 +index abc7495..ae16f4b 100644 --- a/meson_options.txt +++ b/meson_options.txt -@@ -13,3 +13,15 @@ option( +@@ -10,3 +10,15 @@ option( option( 'fru-device-resizefru', value : false, type: 'boolean', description: 'Allow FruDevice to resize FRU areas.', ) @@ -52,7 +52,7 @@ +) \ No newline at end of file diff --git a/src/fru_device.cpp b/src/fru_device.cpp -index 284e123..740a539 100644 +index d1c62fa..3b10c41 100644 --- a/src/fru_device.cpp +++ b/src/fru_device.cpp @@ -712,10 +712,12 @@ void loadBlocklist(const char* path) @@ -71,7 +71,7 @@ for (const auto& i2cBus : i2cBuses) { int bus = busStrToInt(i2cBus.string()); -@@ -783,14 +785,18 @@ static void findI2CDevices(const std::vector<fs::path>& i2cBuses, +@@ -783,41 +785,77 @@ static void findI2CDevices(const std::vector<fs::path>& i2cBuses, } // fd is closed in this function in case the bus locks up @@ -92,40 +92,47 @@ } // this class allows an async response after all i2c devices are discovered -@@ -800,17 +806,43 @@ struct FindDevicesWithCallback : - FindDevicesWithCallback(const std::vector<fs::path>& i2cBuses, - BusMap& busmap, const bool& powerIsOn, + struct FindDevicesWithCallback : + std::enable_shared_from_this<FindDevicesWithCallback> + { +- FindDevicesWithCallback(const std::vector<fs::path>& i2cBuses, +- BusMap& busmap, const bool& powerIsOn, ++ FindDevicesWithCallback(std::vector<fs::path>&& i2cBuses, BusMap& busmap, ++ const bool& powerIsOn, sdbusplus::asio::object_server& objServer, - std::function<void(void)>&& callback) : +- _i2cBuses(i2cBuses), _busMap(busmap), _powerIsOn(powerIsOn), +- _objServer(objServer), _callback(std::move(callback)) + std::function<void()>&& callback, + size_t retries = maxRetries) : - _i2cBuses(i2cBuses), _busMap(busmap), _powerIsOn(powerIsOn), -- _objServer(objServer), _callback(std::move(callback)) ++ _i2cBuses(std::move(i2cBuses)), _busMap(busmap), _powerIsOn(powerIsOn), + _objServer(objServer), _callback(std::move(callback)), _retries(retries) {} ~FindDevicesWithCallback() { - _callback(); +- _callback(); + if (_retryI2cBuses.empty()) + { + std::cerr << "All I2C devices discovered successfully.\n"; ++ _callback(); + return; + } + if (maxRetries == 0) + { ++ _callback(); + return; + } + if (_retries == 0) + { -+ std::cerr << -+ "Failed to discover all I2C devices after " << maxRetries << " retries."; -+ ++ std::cerr << "Failed to discover all I2C devices after " ++ << maxRetries << " retries."; ++ _callback(); + return; + } + + auto scan = std::make_shared<FindDevicesWithCallback>( -+ _i2cBuses, _busMap, _powerIsOn, _objServer, std::move(_callback), -+ _retries - 1); ++ std::move(_i2cBuses), _busMap, _powerIsOn, _objServer, ++ std::move(_callback), _retries - 1); + auto timer = std::make_shared<boost::asio::steady_timer>(io); + timer->expires_after(retryDelay); + timer->async_wait( @@ -138,8 +145,9 @@ + _objServer); } - const std::vector<fs::path>& _i2cBuses; -@@ -818,6 +850,11 @@ struct FindDevicesWithCallback : +- const std::vector<fs::path>& _i2cBuses; ++ std::vector<fs::path> _i2cBuses; + BusMap& _busMap; const bool& _powerIsOn; sdbusplus::asio::object_server& _objServer; std::function<void(void)> _callback; @@ -151,8 +159,26 @@ }; void addFruObjectToDbus( +@@ -1094,7 +1132,7 @@ void rescanOneBus( + i2cBuses.emplace_back(busPath); + + auto scan = std::make_shared<FindDevicesWithCallback>( +- i2cBuses, busmap, powerIsOn, objServer, ++ std::move(i2cBuses), busmap, powerIsOn, objServer, + [busNum, &busmap, &dbusInterfaceMap, &unknownBusObjectCount, &powerIsOn, + &objServer, &systemBus]() { + for (auto busIface = dbusInterfaceMap.begin(); +@@ -1175,7 +1213,7 @@ void rescanBusses( + foundDevices.clear(); + + auto scan = std::make_shared<FindDevicesWithCallback>( +- i2cBuses, busmap, powerIsOn, objServer, [&]() { ++ std::move(i2cBuses), busmap, powerIsOn, objServer, [&]() { + for (auto& busIface : dbusInterfaceMap) + { + objServer.remove_interface(busIface.second); diff --git a/src/meson.build b/src/meson.build -index 9fa3b9c..1ba7cd1 100644 +index 445aef2..0342fab 100644 --- a/src/meson.build +++ b/src/meson.build @@ -20,11 +20,23 @@ executable( @@ -180,5 +206,5 @@ 'fru-device', 'expression.cpp', -- -2.53.0.rc1.225.gd81095ad13-goog +2.54.0.1136.gdb2ca164c4-goog
diff --git a/recipes-phosphor/configuration/entity-manager/0004-fru-device-Reuse-old-dbus-path-if-possible.patch b/recipes-phosphor/configuration/entity-manager/0004-fru-device-Reuse-old-dbus-path-if-possible.patch index 2a3c87b..746069b 100644 --- a/recipes-phosphor/configuration/entity-manager/0004-fru-device-Reuse-old-dbus-path-if-possible.patch +++ b/recipes-phosphor/configuration/entity-manager/0004-fru-device-Reuse-old-dbus-path-if-possible.patch
@@ -1,7 +1,7 @@ -From cec64a9aa9f81536615f1ad653cadafa39d1a347 Mon Sep 17 00:00:00 2001 +From 41a3f47376da6ead5bc9ca16f6b554da6059e626 Mon Sep 17 00:00:00 2001 From: Willy Tu <wltu@google.com> Date: Wed, 21 Jan 2026 07:50:13 +0000 -Subject: [PATCH 4/5] fru-device: Reuse old dbus path if possible +Subject: [PATCH] fru-device: Reuse old dbus path if possible There is a race condition between Entity Manager where call GetSubTree for `xyz.openbmc_project.FruDevice` interfaces based on the Probe @@ -42,10 +42,10 @@ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/fru_device.cpp b/src/fru_device.cpp -index 72e9889..5a3e074 100644 +index 3b10c41..8e74326 100644 --- a/src/fru_device.cpp +++ b/src/fru_device.cpp -@@ -861,7 +861,9 @@ void addFruObjectToDbus( +@@ -865,7 +865,9 @@ void addFruObjectToDbus( std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap, uint32_t bus, uint32_t address, size_t& unknownBusObjectCount, const bool& powerIsOn, sdbusplus::asio::object_server& objServer, @@ -56,7 +56,7 @@ { boost::container::flat_map<std::string, std::string> formattedFRU; -@@ -876,16 +878,25 @@ void addFruObjectToDbus( +@@ -880,16 +882,25 @@ void addFruObjectToDbus( std::string productName = "/xyz/openbmc_project/FruDevice/" + optionalProductName.value(); @@ -88,7 +88,7 @@ for (auto& property : formattedFRU) { -@@ -1101,11 +1112,13 @@ void rescanOneBus( +@@ -1103,11 +1114,13 @@ void rescanOneBus( sdbusplus::asio::object_server& objServer, std::shared_ptr<sdbusplus::asio::connection>& systemBus) { @@ -102,16 +102,16 @@ device = foundDevices.erase(device); } else -@@ -1132,7 +1145,7 @@ void rescanOneBus( +@@ -1134,7 +1147,7 @@ void rescanOneBus( auto scan = std::make_shared<FindDevicesWithCallback>( - i2cBuses, busmap, powerIsOn, objServer, + std::move(i2cBuses), busmap, powerIsOn, objServer, [busNum, &busmap, &dbusInterfaceMap, &unknownBusObjectCount, &powerIsOn, - &objServer, &systemBus]() { + &objServer, &systemBus, oldNames{std::move(oldNames)}]() { for (auto busIface = dbusInterfaceMap.begin(); busIface != dbusInterfaceMap.end();) { -@@ -1156,7 +1169,7 @@ void rescanOneBus( +@@ -1158,7 +1171,7 @@ void rescanOneBus( addFruObjectToDbus(device.second, dbusInterfaceMap, static_cast<uint32_t>(busNum), device.first, unknownBusObjectCount, powerIsOn, objServer, @@ -121,5 +121,5 @@ }); scan->run(); -- -2.53.0.rc1.225.gd81095ad13-goog +2.54.0.1136.gdb2ca164c4-goog