From d46842e52fcf39c5c9248b41c12cd9ca29e51b50 Mon Sep 17 00:00:00 2001 From: paneendrakumar0 Date: Mon, 26 Oct 2026 10:17:22 +0530 Subject: [PATCH] Avoid using srand/rand for client_key generation (#324) In newlib-based embedded environments (e.g. bare-metal ARM Cortex-M, FreeRTOS, Zephyr, and STM32/ESP targets), calling srand() internally invokes malloc() to initialize reentrant state. On systems where dynamic memory allocation is disabled, unconfigured, or forbidden in safety-critical contexts, this causes an abort or memory allocation fault during rmw_init_options_init. Additionally, standard rand() only generates values up to RAND_MAX (often 32767 on embedded toolchains), leaving the upper bits of the 32-bit client_key unset. Replace srand() and rand() with a lightweight, zero-allocation 32-bit Marsaglia xorshift pseudo-random generator seeded with uxr_nanos() to generate the XRCE client_key without dynamic memory allocation. Signed-off-by: paneendrakumar0 --- rmw_microxrcedds_c/src/rmw_init.c | 27 ++++++++++++++++++++++----- rmw_microxrcedds_c/test/test_rmw.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/rmw_microxrcedds_c/src/rmw_init.c b/rmw_microxrcedds_c/src/rmw_init.c index 332d1eda..7bc966eb 100644 --- a/rmw_microxrcedds_c/src/rmw_init.c +++ b/rmw_microxrcedds_c/src/rmw_init.c @@ -35,6 +35,27 @@ extern rmw_uxrce_transport_params_t rmw_uxrce_transport_default_params; +static uint32_t +generate_client_key(void) +{ + static uint32_t state = 0; + int64_t now = uxr_nanos(); + uint32_t entropy = (uint32_t)(now ^ (now >> 32)); + state ^= entropy; + if (state == 0) { + state = 0x1337BEEF; + } + uint32_t key; + do { + // 32-bit Marsaglia xorshift PRNG (zero dynamic memory allocation) + state ^= state << 13; + state ^= state >> 17; + state ^= state << 5; + key = state; + } while (key == 0); + return key; +} + rmw_ret_t rmw_init_options_init( rmw_init_options_t * init_options, @@ -108,11 +129,7 @@ rmw_init_options_init( init_options->impl->transport_params.read_cb = rmw_uxrce_transport_default_params.read_cb; #endif /* if defined(RMW_UXRCE_TRANSPORT_SERIAL) */ - srand(uxr_nanos()); - - do { - init_options->impl->transport_params.client_key = rand(); //NOLINT - } while (init_options->impl->transport_params.client_key == 0); + init_options->impl->transport_params.client_key = generate_client_key(); return RMW_RET_OK; } diff --git a/rmw_microxrcedds_c/test/test_rmw.cpp b/rmw_microxrcedds_c/test/test_rmw.cpp index 1a8a954f..ca25ee0f 100644 --- a/rmw_microxrcedds_c/test/test_rmw.cpp +++ b/rmw_microxrcedds_c/test/test_rmw.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -92,3 +93,28 @@ TEST(rmw_microxrcedds, sync_session) ASSERT_EQ(rmw_init_options_fini(&test_options), RMW_RET_OK); ASSERT_EQ(rmw_shutdown(&test_context), RMW_RET_OK); } + +/* + * Testing client_key generation in rmw_init_options_init. https://github.com/micro-ROS/rmw_microxrcedds/issues/324 + */ +TEST(rmw_microxrcedds, client_key_generation) +{ + rmw_init_options_t options1 = rmw_get_zero_initialized_init_options(); + rmw_init_options_t options2 = rmw_get_zero_initialized_init_options(); + + ASSERT_EQ(rmw_init_options_init(&options1, rcutils_get_default_allocator()), RMW_RET_OK); + ASSERT_EQ(rmw_init_options_init(&options2, rcutils_get_default_allocator()), RMW_RET_OK); + + EXPECT_NE(options1.impl->transport_params.client_key, 0u); + EXPECT_NE(options2.impl->transport_params.client_key, 0u); + EXPECT_NE(options1.impl->transport_params.client_key, options2.impl->transport_params.client_key); + + rmw_init_options_t options_copy = rmw_get_zero_initialized_init_options(); + ASSERT_EQ(rmw_init_options_copy(&options1, &options_copy), RMW_RET_OK); + EXPECT_EQ(options_copy.impl->transport_params.client_key, + options1.impl->transport_params.client_key); + + ASSERT_EQ(rmw_init_options_fini(&options_copy), RMW_RET_OK); + ASSERT_EQ(rmw_init_options_fini(&options1), RMW_RET_OK); + ASSERT_EQ(rmw_init_options_fini(&options2), RMW_RET_OK); +}