From 34ff721c92f5a2948a82d72d40af2a40a9f532b0 Mon Sep 17 00:00:00 2001 From: tgotic Date: Wed, 3 Aug 2022 14:31:32 +0200 Subject: [PATCH 1/2] [gpio] calloc in critical section Remove calloc out of critical section. In critical section, assign allocated memory to gpio_isr_func. Free resources if gpio_isr_register() fails. --- components/driver/gpio/gpio.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/components/driver/gpio/gpio.c b/components/driver/gpio/gpio.c index 0abbbeb5e0..8221b7f194 100644 --- a/components/driver/gpio/gpio.c +++ b/components/driver/gpio/gpio.c @@ -447,14 +447,17 @@ static void IRAM_ATTR gpio_intr_service(void *arg) esp_err_t gpio_install_isr_service(int intr_alloc_flags) { GPIO_CHECK(gpio_context.gpio_isr_func == NULL, "GPIO isr service already installed", ESP_ERR_INVALID_STATE); - esp_err_t ret; - portENTER_CRITICAL(&gpio_context.gpio_spinlock); - gpio_context.gpio_isr_func = (gpio_isr_func_t *) calloc(GPIO_NUM_MAX, sizeof(gpio_isr_func_t)); - portEXIT_CRITICAL(&gpio_context.gpio_spinlock); - if (gpio_context.gpio_isr_func == NULL) { - ret = ESP_ERR_NO_MEM; - } else { + esp_err_t ret = ESP_ERR_NO_MEM; + gpio_isr_func_t *isr_func = (gpio_isr_func_t *) calloc(GPIO_NUM_MAX, sizeof(gpio_isr_func_t)); + if (isr_func) { + portENTER_CRITICAL(&gpio_context.gpio_spinlock); + gpio_context.gpio_isr_func = isr_func; + portEXIT_CRITICAL(&gpio_context.gpio_spinlock); ret = gpio_isr_register(gpio_intr_service, NULL, intr_alloc_flags, &gpio_context.gpio_isr_handle); + if (ret != ESP_OK) { + // registering failed, free allocated resources + gpio_uninstall_isr_service(); + } } return ret; From 6e8ebdb6b7589540bef70617ce812dd3b0d55ae3 Mon Sep 17 00:00:00 2001 From: songruojing Date: Thu, 4 Aug 2022 17:38:54 +0800 Subject: [PATCH 2/2] gpio: fix potential race condition inside gpio_install_isr_service --- components/driver/gpio/gpio.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/components/driver/gpio/gpio.c b/components/driver/gpio/gpio.c index 8221b7f194..2c95fce523 100644 --- a/components/driver/gpio/gpio.c +++ b/components/driver/gpio/gpio.c @@ -451,12 +451,19 @@ esp_err_t gpio_install_isr_service(int intr_alloc_flags) gpio_isr_func_t *isr_func = (gpio_isr_func_t *) calloc(GPIO_NUM_MAX, sizeof(gpio_isr_func_t)); if (isr_func) { portENTER_CRITICAL(&gpio_context.gpio_spinlock); - gpio_context.gpio_isr_func = isr_func; - portEXIT_CRITICAL(&gpio_context.gpio_spinlock); - ret = gpio_isr_register(gpio_intr_service, NULL, intr_alloc_flags, &gpio_context.gpio_isr_handle); - if (ret != ESP_OK) { - // registering failed, free allocated resources - gpio_uninstall_isr_service(); + if (gpio_context.gpio_isr_func == NULL) { + gpio_context.gpio_isr_func = isr_func; + portEXIT_CRITICAL(&gpio_context.gpio_spinlock); + ret = gpio_isr_register(gpio_intr_service, NULL, intr_alloc_flags, &gpio_context.gpio_isr_handle); + if (ret != ESP_OK) { + // registering failed, uninstall isr service + gpio_uninstall_isr_service(); + } + } else { + // isr service already installed, free allocated resource + portEXIT_CRITICAL(&gpio_context.gpio_spinlock); + ret = ESP_ERR_INVALID_STATE; + free(isr_func); } }