Skip to content

Commit ff16585

Browse files
committed
hal_lib: fix a race in hal_init()
This commit makes two changes in init_hal_data() (which is called by hal_init()): 1. Take the `hal_data->mutex` using `rtapi_mutex_get()` instead of `rtapi_mutex_try()`. The `try` version tries to take the mutex and returns immediately, indicating by its return value whether it actually took the mutex or not. Before this commit, the `init_hal_data()` function called `try` but didn't check the return value, and just assumed that it got the mutex. This commit switches to the `get` version, which blocks until the mutex is available, then takes it. Note: this relies on the promise made by rtapi_shmem_new() that a freshly-allocated `hal_data` is initialized such that `hal_data->mutex` is a valid, un-locked mutex. Uspace RTAPI meets this criterion because it uses shmget(), which initializes all shared memory to all-bytes-zero. RTAI RTAPI also meets this criterion, though it's not mentioned in the documentation: https://mail.rtai.org/pipermail/rtai/2022-January/028282.html http://cvs.savannah.nongnu.org/viewvc/rtai/magma/base/ipc/shm/shm.c?view=markup#l69 2. Take the `hal_data->mutex`, locking `hal_data`, *before* checking whether the memory is initialized. This fixes the hal_init() race condition.
1 parent 0fe6bab commit ff16585

1 file changed

Lines changed: 15 additions & 4 deletions

File tree

src/hal/hal_lib.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2781,18 +2781,29 @@ static void thread_task(void *arg)
27812781

27822782
static int init_hal_data(void)
27832783
{
2784-
/* has the block already been initialized? */
2784+
/* has the hal_data block already been initialized? */
2785+
2786+
/* Lock hal_data by taking the mutex, so that two processes
2787+
don't both try to initialize hal_data at the same time. NOTE:
2788+
The first time through, the hal_data memory buffer is fresh from
2789+
rtapi_shmem_new(), which means it's initialized to all zero bytes.
2790+
This means hal_data->mutex is valid and unlocked. */
2791+
rtapi_mutex_get(&(hal_data->mutex));
2792+
27852793
if (hal_data->version != 0) {
2786-
/* yes, verify version code */
2794+
/* hal_data has been initialized already, verify version code */
27872795
if (hal_data->version == HAL_VER) {
2796+
rtapi_mutex_give(&(hal_data->mutex));
27882797
return 0;
27892798
} else {
27902799
rtapi_print_msg(RTAPI_MSG_ERR, "HAL: ERROR: version code mismatch\n");
2800+
rtapi_mutex_give(&(hal_data->mutex));
27912801
return -1;
27922802
}
27932803
}
2794-
/* no, we need to init it, grab the mutex unconditionally */
2795-
rtapi_mutex_try(&(hal_data->mutex));
2804+
2805+
/* hal_data has *NOT* been initialized yet, we get the honor */
2806+
27962807
/* set version code so nobody else init's the block */
27972808
hal_data->version = HAL_VER;
27982809
/* initialize everything */

0 commit comments

Comments
 (0)