Skip to content

nrf_rpc: fix possible data race at initialization #1577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 20, 2024
Merged

nrf_rpc: fix possible data race at initialization #1577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb95a10
nrf_rpc: remove unused APIs from OS template
Damian-Nordic Nov 19, 2024
16b7bd2
nrf_rpc: fix possible data race at initialization
Damian-Nordic Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions nrf_rpc/nrf_rpc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,19 +414,22 @@ static int transport_init(nrf_rpc_tr_receive_handler_t receive_cb)

NRF_RPC_ASSERT(transport != NULL);

err = transport->api->init(transport, receive_cb, NULL);
if (err) {
NRF_RPC_ERR("Failed to initialize transport, err: %d", err);
continue;
}

/* Initialize all dependencies of `receive_handler` before calling the transport
* init to avoid possible data race if `receive_handler` was invoked before this
* function was completed. */
if (auto_free_rx_buf(transport)) {
err = nrf_rpc_os_event_init(&data->decode_done_event);
if (err < 0) {
continue;
}
}

err = transport->api->init(transport, receive_cb, NULL);
if (err) {
NRF_RPC_ERR("Failed to initialize transport, err: %d", err);
continue;
}

group->data->transport_initialized = true;

if (group->flags & NRF_RPC_FLAGS_INITIATOR) {
Expand Down Expand Up @@ -694,7 +697,7 @@ static int init_packet_handle(struct header *hdr, const struct nrf_rpc_group **g
* either we are not an initiator, which is indicated by NRF_RPC_FLAGS_INITIATOR
* flag, or the remote has missed our init packet.
*/
send_reply = (hdr->dst_group_id == NRF_RPC_ID_UNKNOWN) && group_data->transport_initialized;
send_reply = (hdr->dst_group_id == NRF_RPC_ID_UNKNOWN);

/*
* Spawn the async task only if necessary. The async task is used to avoid sending the init
Expand Down Expand Up @@ -742,6 +745,12 @@ static void receive_handler(const struct nrf_rpc_tr *transport, const uint8_t *p
err = -NRF_EBADMSG;
goto cleanup_and_exit;
}

/* Mark the transport as initialized in case the first packet arrives sooner than
* `nrf_rpc_init` does that. Without this, an attempt to allocate a buffer for the
* response to this packet would fail.
*/
group->data->transport_initialized = true;
}

NRF_RPC_DBG("Received %d bytes packet from %d to %d, type 0x%02X, "
Expand Down
23 changes: 0 additions & 23 deletions nrf_rpc/template/nrf_rpc_os_tmpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,29 +156,6 @@ uint32_t nrf_rpc_os_ctx_pool_reserve();
*/
void nrf_rpc_os_ctx_pool_release(uint32_t index);

/** @brief Set number of remote threads.
*
* Number of remote threads that can be reserved by
* @ref nrf_rpc_os_remote_reserve is limited by `count` parameter.
* After initialization `count` is assumed to be zero.
*
* @param count Number of remote threads.
*/
void nrf_rpc_os_remote_count(int count);

/** @brief Reserve one thread from a remote thread pool.
*
* If there are no more threads available or @ref nrf_rpc_os_remote_count was
* not called yet then this function waits.
*
* Remote thread reserving and releasing can be implemented using a semaphore.
*/
void nrf_rpc_os_remote_reserve();

/** @brief Release one thread from a remote thread pool.
*/
void nrf_rpc_os_remote_release();

#ifdef __cplusplus
}
#endif
Expand Down
Loading