RouDi crash in publishServiceRegistry when assigning to unzeroized allocated chunk

[afrixs]

Required information

Operating system:
E.g. Ubuntu 22.04 LTS

Compiler version:
E.g. GCC 11.2.0

Observed result or behaviour:

Condition: has_value() in T& iox::cxx::optional<T>::value() & [with T = iox::roudi::ServiceRegistry::ServiceDescriptionEntry] is violated. (/home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/optional.inl:262)
2022-08-12 17:35:46.978 [ Error ]: ICEORYX error! EXPECTS_ENSURES_FAILED
iox-roudi: /home/crawler/r2_vine/dds_ws/src/iceoryx/iceoryx_hoofs/source/error_handling/error_handler.cpp:39: static void iox::ErrorHandler::reactOnErrorLevel(iox::ErrorLevel, const char*): Assertion `false' failed.

> bt
...
#12 0x00007ffff750b99f in iox::cxx::internal::Require (condition=49,
    file=0x7ffff7841500 "/home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/optional.inl", line=262,
    function=0x7ffff7844158 "T& iox::cxx::optional<T>::value() & [with T = iox::roudi::ServiceRegistry::ServiceDescriptionEntry]",
    conditionString=0x7ffff78414a9 "has_value()") at /home/crawler/r2_vine/dds_ws/src/iceoryx/iceoryx_hoofs/source/cxx/requires.cpp:38
#13 0x00007ffff7813657 in iox::cxx::optional<iox::roudi::ServiceRegistry::ServiceDescriptionEntry>::value() & (this=0x7fffe966dbf0)
    at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/optional.inl:262
#14 0x00007ffff7811bc6 in iox::cxx::optional<iox::roudi::ServiceRegistry::ServiceDescriptionEntry>::operator= (this=0x7fffe966dbf0, rhs=...)
    at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/optional.inl:104
#15 0x00007ffff780f5e4 in iox::cxx::vector<iox::cxx::optional<iox::roudi::ServiceRegistry::ServiceDescriptionEntry>, 4096ul>::operator= (this=0x7fffe966c6f0,
    rhs=...) at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/vector.inl:96
#16 0x00007ffff780bf2d in iox::roudi::ServiceRegistry::operator= (this=0x7fffe966c6f0)
    at /home/crawler/r2_vine/dds_ws/src/iceoryx/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp:35
#17 0x00007ffff78044ad in operator()<iox::mepoo::ChunkHeader*> (__closure=0x7fffde2ca8f0, chunk=@0x7fffde2ca920: 0x7fffe966c6c8)
    at /home/crawler/r2_vine/dds_ws/src/iceoryx/iceoryx_posh/source/roudi/port_manager.cpp:1057
#18 0x00007ffff780579c in operator() (__closure=0x0, target=0x7fffde2ca8f0, args#0=@0x7fffde2ca920: 0x7fffe966c6c8)
    at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/function_ref.inl:46
#19 0x00007ffff78057cc in _FUN () at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/function_ref.inl:41
#20 0x00007ffff780f76b in iox::cxx::function_ref<void (iox::mepoo::ChunkHeader*&)>::operator()(iox::mepoo::ChunkHeader*&) const (this=0x7fffde2ca880,
    args#0=@0x7fffde2ca920: 0x7fffe966c6c8)
    at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/function_ref.inl:100
#21 0x00007ffff7804532 in iox::cxx::internal::AndThenWithValue<iox::cxx::expected<iox::mepoo::ChunkHeader*, iox::popo::AllocationError>, iox::mepoo::ChunkHeader*>::and_then<iox::roudi::PortManager::publishServiceRegistry() const::<lambda(auto:27&)> >(const struct {...} &) (this=0x7fffde2ca920, callable=...)
    at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/functional_interface.inl:138
#22 0x00007ffff7802afd in iox::cxx::internal::AndThenWithValue<iox::cxx::expected<iox::mepoo::ChunkHeader*, iox::popo::AllocationError>, iox::mepoo::ChunkHeader*>::and_then<iox::roudi::PortManager::publishServiceRegistry() const::<lambda(auto:27&)> >(const struct {...} &) (this=0x7fffde2ca920, callable=...)
    at /home/crawler/r2_vine/dds_ws/install/iceoryx_hoofs/include/iceoryx/v2.90.0/iceoryx_hoofs/internal/cxx/functional_interface.inl:148
#23 0x00007ffff78023e0 in iox::roudi::PortManager::publishServiceRegistry (this=0x7ffff7885ad0 <iox::roudi::IceOryxRouDiApp::run()::m_rouDiComponents+40016>)
    at /home/crawler/r2_vine/dds_ws/src/iceoryx/iceoryx_posh/source/roudi/port_manager.cpp:1053
#24 0x00007ffff78024ab in iox::roudi::PortManager::addPublisherToServiceRegistry (
    this=0x7ffff7885ad0 <iox::roudi::IceOryxRouDiApp::run()::m_rouDiComponents+40016>, service=...)
    at /home/crawler/r2_vine/dds_ws/src/iceoryx/iceoryx_posh/source/roudi/port_manager.cpp:1075
#25 0x00007ffff7802c0c in operator()<iox::capro::CaproMessage> (__closure=0x7fffde2cad10, caproMessage=...)
    at /home/crawler/r2_vine/dds_ws/src/iceoryx/iceoryx_posh/source/roudi/port_manager.cpp:144
...

The backtrace shows that has_value() of the ServiceRegistry optional entry being assigned to is neither true nor false but 49. The reason is that the sample is freshly allocated and its data are undefined. I believe there should be some simple "clearing" of the allocated struct before assigning real data to by-pass previous data deallocation/optional validity checks, because previous data are not real. This is what worked for us as a hotfix:

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/vector.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/vector.hpp
index 843977328..222930913 100644
--- a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/vector.hpp
+++ b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/vector.hpp
@@ -157,6 +157,11 @@ class vector
     /// @brief calls the destructor of all contained elements and removes them
     void clear() noexcept;

+    /// @brief sets size to 0, not calling destructor of elements
+    /// @note should be called (only) after static-casting externally allocated bytes
+    ///       to the vector, resulting in its size value being undefined
+    void setEmpty() noexcept;
+
     /// @brief resizes the vector. If the vector size increases new elements
     /// will be constructed with the given arguments. If count is greater than the capacity
     /// the vector will stay unchanged.
diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/vector.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/vector.inl
index eccbb9e6e..7f61bb2c0 100644
--- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/vector.inl
+++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/vector.inl
@@ -169,6 +169,12 @@ inline void vector<T, Capacity>::clear() noexcept
     }
 }

+template <typename T, uint64_t Capacity>
+inline void vector<T, Capacity>::setEmpty() noexcept
+{
+    m_size = 0U;
+}
+
 template <typename T, uint64_t Capacity>
 template <typename... Targs>
 inline bool vector<T, Capacity>::emplace_back(Targs&&... args) noexcept
diff --git a/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp b/iceoryx_posh/include/iceoryx_posh/internal/roudi/servic>
index 06daeb4aa..7931ae59b 100644
--- a/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp
+++ b/iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp
@@ -96,6 +96,14 @@ class ServiceRegistry
     /// @note Can be used to obtain all entries or count them
     void forEach(cxx::function_ref<void(const ServiceDescriptionEntry&)> callable) const noexcept;

+    /// @brief clears itself without calling any destructors
+    /// @note should be called (only) after static-casting externally allocated bytes
+    ///       to the ServiceRegistry, resulting in its data being undefined
+    void setEmpty() {
+      m_serviceDescriptions.setEmpty();
+      m_freeIndex = NO_INDEX;
+    }
+
   private:
     using Entry_t = cxx::optional<ServiceDescriptionEntry>;
     using ServiceDescriptionContainer_t = cxx::vector<Entry_t, CAPACITY>;
diff --git a/iceoryx_posh/source/roudi/port_manager.cpp b/iceoryx_posh/source/roudi/port_manager.cpp
index 753469d00..da3d84d91 100644
--- a/iceoryx_posh/source/roudi/port_manager.cpp
+++ b/iceoryx_posh/source/roudi/port_manager.cpp
@@ -1052,6 +1052,7 @@ void PortManager::publishServiceRegistry() const noexcept
                           CHUNK_NO_USER_HEADER_ALIGNMENT)
         .and_then([&](auto& chunk) {
             auto sample = static_cast<ServiceRegistry*>(chunk->userPayload());
+            sample->setEmpty();

             // It's ok to copy as the modifications happen in the same thread and not concurrently
             *sample = m_serviceRegistry;

Expected result or behaviour:
To proceed without crash.

Conditions where it occurred / Performed steps:
Quite heavy load, >100 ROS2 nodes, >512 publishers and subscribers, iceoryx_posh was built with IOX_MAX_PUBLISHERS=1024

[elBoberido]

@mossmaurice can you have a look at this? I think the proper solution is to do a placement new on the chunk payload in PortManager::publishServiceRegistry() like

new (chunk->userPayload()) (m_serviceRegistry);

[afrixs]

Confirming that

        .and_then([&](auto& chunk) {
            // It's ok to copy as the modifications happen in the same thread and not concurrently
            new (chunk->userPayload()) ServiceRegistry(m_serviceRegistry);

            publisher.sendChunk(chunk);
        })

works without the crash

[mossmaurice]

@afrixs Great, thanks for testing! I will create a bugfix PR in the next days.