Skip to content

Commit 142c389

Browse files
committed
Add mutex to avoid race conditions between timer callback and API
The timer callback and public API methods (enable/disable acceleration, delete) access shared state. This change adds a FreeRTOS mutex to protect against torn reads of the 64-bit last_time field on 32-bit platforms and use-after-free when deleting an encoder while its timer callback is in-flight.
1 parent 01ac602 commit 142c389

1 file changed

Lines changed: 27 additions & 1 deletion

File tree

encoder.c

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
#include <string.h>
4040
#include <esp_log.h>
4141
#include <esp_timer.h>
42+
#include <freertos/FreeRTOS.h>
43+
#include <freertos/semphr.h>
4244

4345
#if defined(CONFIG_IDF_TARGET_ESP8266) && CONFIG_RE_INTERVAL_US < 10000
4446
#error Too small CONFIG_RE_INTERVAL_US! For ESP8266 it should be >= 10000
@@ -71,6 +73,7 @@ struct rotary_encoder
7173
rotary_encoder_event_cb_t callback; //!< Event callback
7274
void *callback_ctx; //!< User context passed to callback
7375
esp_timer_handle_t timer;
76+
SemaphoreHandle_t lock;
7477
uint8_t code;
7578
uint16_t store;
7679
uint64_t btn_pressed_time_us;
@@ -178,7 +181,10 @@ inline static void read_encoder(rotary_encoder_handle_t handle)
178181

179182
static void encoder_timer_handler(void *arg)
180183
{
181-
read_encoder((rotary_encoder_handle_t)arg);
184+
rotary_encoder_handle_t handle = (rotary_encoder_handle_t)arg;
185+
xSemaphoreTake(handle->lock, portMAX_DELAY);
186+
read_encoder(handle);
187+
xSemaphoreGive(handle->lock);
182188
}
183189

184190
esp_err_t rotary_encoder_create(const rotary_encoder_config_t *config, rotary_encoder_handle_t *handle)
@@ -201,10 +207,18 @@ esp_err_t rotary_encoder_create(const rotary_encoder_config_t *config, rotary_en
201207
re->callback = config->callback;
202208
re->callback_ctx = config->callback_ctx;
203209

210+
re->lock = xSemaphoreCreateMutex();
211+
if (!re->lock)
212+
{
213+
free(re);
214+
return ESP_ERR_NO_MEM;
215+
}
216+
204217
#if defined(CONFIG_IDF_TARGET_ESP8266)
205218
if (re->polling_interval_us < 10000)
206219
{
207220
ESP_LOGE(TAG, "Polling interval too small for ESP8266, must be >= 10000us");
221+
vSemaphoreDelete(re->lock);
208222
free(re);
209223
return ESP_ERR_INVALID_ARG;
210224
}
@@ -240,6 +254,7 @@ esp_err_t rotary_encoder_create(const rotary_encoder_config_t *config, rotary_en
240254
esp_err_t err = gpio_config(&io_conf);
241255
if (err != ESP_OK)
242256
{
257+
vSemaphoreDelete(re->lock);
243258
free(re);
244259
return err;
245260
}
@@ -256,6 +271,7 @@ esp_err_t rotary_encoder_create(const rotary_encoder_config_t *config, rotary_en
256271
err = esp_timer_create(&timer_args, &re->timer);
257272
if (err != ESP_OK)
258273
{
274+
vSemaphoreDelete(re->lock);
259275
free(re);
260276
return err;
261277
}
@@ -264,6 +280,7 @@ esp_err_t rotary_encoder_create(const rotary_encoder_config_t *config, rotary_en
264280
if (err != ESP_OK)
265281
{
266282
esp_timer_delete(re->timer);
283+
vSemaphoreDelete(re->lock);
267284
free(re);
268285
return err;
269286
}
@@ -281,6 +298,11 @@ esp_err_t rotary_encoder_delete(rotary_encoder_handle_t handle)
281298
esp_timer_stop(handle->timer);
282299
esp_timer_delete(handle->timer);
283300

301+
// Wait for any in-flight callback to complete before freeing
302+
xSemaphoreTake(handle->lock, portMAX_DELAY);
303+
xSemaphoreGive(handle->lock);
304+
vSemaphoreDelete(handle->lock);
305+
284306
ESP_LOGI(TAG, "Deleted rotary encoder, A: %d, B: %d, BTN: %d", handle->pin_a, handle->pin_b, handle->pin_btn);
285307
free(handle);
286308
return ESP_OK;
@@ -289,14 +311,18 @@ esp_err_t rotary_encoder_delete(rotary_encoder_handle_t handle)
289311
esp_err_t rotary_encoder_enable_acceleration(rotary_encoder_handle_t handle, uint16_t coeff)
290312
{
291313
CHECK_ARG(handle);
314+
xSemaphoreTake(handle->lock, portMAX_DELAY);
292315
handle->acceleration.coeff = coeff;
293316
handle->acceleration.last_time = esp_timer_get_time();
317+
xSemaphoreGive(handle->lock);
294318
return ESP_OK;
295319
}
296320

297321
esp_err_t rotary_encoder_disable_acceleration(rotary_encoder_handle_t handle)
298322
{
299323
CHECK_ARG(handle);
324+
xSemaphoreTake(handle->lock, portMAX_DELAY);
300325
handle->acceleration.coeff = 0;
326+
xSemaphoreGive(handle->lock);
301327
return ESP_OK;
302328
}

0 commit comments

Comments
 (0)