michaelh/zephyr
1
0
Fork 0
Code Releases Activity

mgmt: smp: Fix race condition by using same work queue

Browse source 
Fix possible race condition where SMP client might
release the network buffer before system worker queue
has processed it.

In smp_client uses shared resources like worker queue
linked list and network buffers without maintaining any
thread safety.

For unknown reasons, retry timeout handling is pushed
into system worker queue while the actual transmission
is handled from SMP work queue.

Fix the issue by using the same SMP work queue
for both delayable timeout handling as well as
transmission handling.

Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>
This commit is contained in:
Seppo Takalo 2024-12-04 17:14:33 +02:00 • committed by Benjamin Cabé
commit 9fa82f0d8d
3 changed files with 11 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

10
subsys/mgmt/mcumgr/smp_client/src/client.c
View file

@ -112,7 +112,7 @@ static void smp_client_transport_work_fn(struct k_work *work)
entry->retry_cnt--;
entry->timestamp = time_stamp_ref + CONFIG_SMP_CMD_RETRY_TIME;
k_fifo_put(&entry->smp_client->tx_fifo, entry->nb);
smp_tx_req(&entry->smp_client->work);
k_work_submit_to_queue(smp_get_wq(), &entry->smp_client->work);
continue;
}
@ -126,7 +126,8 @@ static void smp_client_transport_work_fn(struct k_work *work)
if (!sys_slist_is_empty(&smp_client_data.cmd_list)) {
/* Re-schedule new timeout to next */
k_work_reschedule(&smp_client_data.work_delay, K_MSEC(backoff_ms));
k_work_reschedule_for_queue(smp_get_wq(), &smp_client_data.work_delay,
K_MSEC(backoff_ms));
}
}
@ -160,7 +161,8 @@ static void smp_cmd_add_to_list(struct smp_client_cmd_req *cmd_req)
{
if (sys_slist_is_empty(&smp_client_data.cmd_list)) {
/* Enable timer */
k_work_reschedule(&smp_client_data.work_delay, K_MSEC(CONFIG_SMP_CMD_RETRY_TIME));
k_work_reschedule_for_queue(smp_get_wq(), &smp_client_data.work_delay,
K_MSEC(CONFIG_SMP_CMD_RETRY_TIME));
}
sys_slist_append(&smp_client_data.cmd_list, &cmd_req->node);
}
@ -320,7 +322,7 @@ int smp_client_send_cmd(struct smp_client_object *smp_client, struct net_buf *nb
nb = net_buf_ref(nb);
smp_cmd_add_to_list(cmd_req);
k_fifo_put(&smp_client->tx_fifo, nb);
smp_tx_req(&smp_client->work);
k_work_submit_to_queue(smp_get_wq(), &smp_client->work);
return MGMT_ERR_EOK;
}

6
subsys/mgmt/mcumgr/transport/include/mgmt/mcumgr/transport/smp_internal.h
View file

@ -50,11 +50,11 @@ void smp_rx_req(struct smp_transport *smtp, struct net_buf *nb);
#ifdef CONFIG_SMP_CLIENT
/**
* @brief Trig SMP client request packet for transmission.
* @brief Get work queue for SMP client.
*
* @param work The transport to use to send the corresponding response(s).
* @return SMP work queue object.
*/
void smp_tx_req(struct k_work *work);
struct k_work_q *smp_get_wq(void);
#endif
/**

4
subsys/mgmt/mcumgr/transport/src/smp.c
View file

@ -204,9 +204,9 @@ smp_rx_req(struct smp_transport *smpt, struct net_buf *nb)
}
#ifdef CONFIG_SMP_CLIENT
void smp_tx_req(struct k_work *work)
struct k_work_q *smp_get_wq(void)
{
k_work_submit_to_queue(&smp_work_queue, work);
return &smp_work_queue;
}
#endif