From 6e7eb3c776cf68669bcc31352ca5493dfb540e6d Mon Sep 17 00:00:00 2001
From: Ivan Grokhotkov <ivan@espressif.com>
Date: Fri, 24 Nov 2017 17:33:13 +0800
Subject: [PATCH] esp_timer: fix for the case when timer is deleted in callback

Timer callback can delete the timer. If CONFIG_ESP_TIMER_PROFILING was
enabled, this caused an access to invalid (freed) memory.
This fix adds a pointer to track the timer while executing the callback.
This is needed so that we can check if callback deletes the timer,
in which case we won't try updating profiling counters for this timer
after the callback is done.
---
 components/esp32/esp_timer.c           | 18 ++++++++++---
 components/esp32/test/test_esp_timer.c | 35 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/components/esp32/esp_timer.c b/components/esp32/esp_timer.c
index 9fd8f72fe7..6987b4c64c 100644
--- a/components/esp32/esp_timer.c
+++ b/components/esp32/esp_timer.c
@@ -75,12 +75,14 @@ static LIST_HEAD(esp_timer_list, esp_timer) s_timers =
 // all the timers
 static LIST_HEAD(esp_inactive_timer_list, esp_timer) s_inactive_timers =
         LIST_HEAD_INITIALIZER(s_timers);
+// used to keep track of the timer when executing the callback
+static esp_timer_handle_t s_timer_in_callback;
 #endif
 // task used to dispatch timer callbacks
 static TaskHandle_t s_timer_task;
 // counting semaphore used to notify the timer task from ISR
 static SemaphoreHandle_t s_timer_semaphore;
-// lock protecting s_timers and s_inactive_timers
+// lock protecting s_timers, s_inactive_timers, s_timer_in_callback
 static portMUX_TYPE s_timer_lock = portMUX_INITIALIZER_UNLOCKED;
 
 
@@ -149,6 +151,9 @@ esp_err_t esp_timer_delete(esp_timer_handle_t timer)
         return ESP_ERR_INVALID_STATE;
     }
 #if WITH_PROFILING
+    if (timer == s_timer_in_callback) {
+        s_timer_in_callback = NULL;
+    }
     timer_remove_inactive(timer);
 #endif
     if (timer == NULL) {
@@ -266,14 +271,21 @@ static void timer_process_alarm(esp_timer_dispatch_t dispatch_method)
         }
 #if WITH_PROFILING
         uint64_t callback_start = now;
+        s_timer_in_callback = it;
 #endif
         timer_list_unlock();
         (*it->callback)(it->arg);
         timer_list_lock();
         now = esp_timer_impl_get_time();
 #if WITH_PROFILING
-        it->times_triggered++;
-        it->total_callback_run_time += now - callback_start;
+        /* The callback might have deleted the timer.
+         * If this happens, esp_timer_delete will set s_timer_in_callback
+         * to NULL.
+         */
+        if (s_timer_in_callback) {
+            s_timer_in_callback->times_triggered++;
+            s_timer_in_callback->total_callback_run_time += now - callback_start;
+        }
 #endif
         it = LIST_FIRST(&s_timers);
     }
diff --git a/components/esp32/test/test_esp_timer.c b/components/esp32/test/test_esp_timer.c
index 08fdb15dc7..d77ddeb5e8 100644
--- a/components/esp32/test/test_esp_timer.c
+++ b/components/esp32/test/test_esp_timer.c
@@ -4,6 +4,7 @@
 #include <sys/time.h>
 #include "unity.h"
 #include "esp_timer.h"
+#include "esp_heap_caps.h"
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "freertos/semphr.h"
@@ -379,3 +380,37 @@ TEST_CASE("Can dump esp_timer stats", "[esp_timer]")
 {
     esp_timer_dump(stdout);
 }
+
+TEST_CASE("Can delete timer from callback", "[esp_timer]")
+{
+    typedef struct {
+        SemaphoreHandle_t notify_from_timer_cb;
+        esp_timer_handle_t timer;
+    } test_arg_t;
+
+    void timer_func(void* varg)
+    {
+        test_arg_t arg = *(test_arg_t*) varg;
+        esp_timer_delete(arg.timer);
+        printf("Timer %p is deleted\n", arg.timer);
+        xSemaphoreGive(arg.notify_from_timer_cb);
+    }
+
+    test_arg_t args = {
+            .notify_from_timer_cb = xSemaphoreCreateBinary(),
+    };
+
+    esp_timer_create_args_t timer_args = {
+            .callback = &timer_func,
+            .arg = &args,
+            .name = "self_deleter"
+    };
+    esp_timer_create(&timer_args, &args.timer);
+    esp_timer_start_once(args.timer, 10000);
+
+    TEST_ASSERT_TRUE(xSemaphoreTake(args.notify_from_timer_cb, 1000 / portTICK_PERIOD_MS));
+    printf("Checking heap at %p\n", args.timer);
+    TEST_ASSERT_TRUE(heap_caps_check_integrity_addr((intptr_t) args.timer, true));
+
+    vSemaphoreDelete(args.notify_from_timer_cb);
+}
-- 
2.40.0