Skip to content

Conversation

rfrandse
Copy link
Collaborator

@rfrandse rfrandse commented Jun 5, 2025

Fix coredump on PCIe Topology Refresh (#1307)

Due to improper lambda capture `&req` [1], its reference inside the
async_method_call later may potentially become invalid if `req` is
already removed. This may cause the coredump of bmcweb at

BMCWEB_LOG_DEBUG << "Set PCIe Topology Refresh status.";
crow::connections::systemBus->async_method_call(
    [&req, aResp](const boost::system::error_code ec) {
    ...
    pcieTopologyRefreshTimer =
        std::make_unique<boost::asio::steady_timer>(*req.ioService);
    ...

The fix is to capture `req` as value so that `req` should be valid
inside the callback.
  •    [&req, asyncResp](const boost::system::error_code& ec) {
    
  •    [req, asyncResp](const boost::system::error_code& ec) {
    

Tested:
- Try setPCIeTopologRefresh multiple times without coredumps

curl -k -H "Content-Type: application/json" -X PATCH https://${bmc}/redfish/v1/Systems/system/ -d '{"Oem":{"IBM":{"PCIeTopologyRefresh":true}}}'


[1] https://github.com/ibm-openbmc/bmcweb/blob/2a42b4b15776e5f6e4777b856c59d0e5c34c7f1e/redfish-core/lib/oem/ibm/pcie_topology_refresh.hpp#L108

Signed-off-by: Myung Bae <[email protected]>```

Due to improper lambda capture `&req` [1], its reference inside the
async_method_call later may potentially become invalid if `req` is
already removed. This may cause the coredump of bmcweb at

````
    BMCWEB_LOG_DEBUG << "Set PCIe Topology Refresh status.";
    crow::connections::systemBus->async_method_call(
        [&req, aResp](const boost::system::error_code ec) {
        ...
        pcieTopologyRefreshTimer =
            std::make_unique<boost::asio::steady_timer>(*req.ioService);
        ...
```

The fix is to capture `req` as value so that `req` should be valid
inside the callback.
```
-        [&req, asyncResp](const boost::system::error_code& ec) {
+        [req, asyncResp](const boost::system::error_code& ec) {
```

Tested:
- Try setPCIeTopologRefresh multiple times without coredumps
```
curl -k -H "Content-Type: application/json" -X PATCH https://${bmc}/redfish/v1/Systems/system/ -d '{"Oem":{"IBM":{"PCIeTopologyRefresh":true}}}'
```

[1] https://github.com/ibm-openbmc/bmcweb/blob/2a42b4b15776e5f6e4777b856c59d0e5c34c7f1e/redfish-core/lib/oem/ibm/pcie_topology_refresh.hpp#L108

Signed-off-by: Myung Bae <[email protected]>
@rfrandse
Copy link
Collaborator Author

rfrandse commented Jun 6, 2025

jenkins run tests please

@gtmills gtmills requested a review from baemyung June 9, 2025 20:42
Copy link
Contributor

@baemyung baemyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me.

(But not sure why CI fails).

@rfrandse
Copy link
Collaborator Author

jenkins run tests please

1 similar comment
@baemyung
Copy link
Contributor

jenkins run tests please

@baemyung
Copy link
Contributor

I think this can be canceled as the fix was already merged - 12dde3f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants