From 1cece7eef0a645c919d68aca487269bdf27cec2b Mon Sep 17 00:00:00 2001
From: morris <maoshengrong@espressif.com>
Date: Mon, 14 Nov 2022 18:46:58 +0800
Subject: [PATCH] rmt: fix memory leak in the legacy driver

Closes https://github.com/espressif/esp-idf/issues/10173
---
 components/driver/rmt.c           | 13 ++++---------
 components/driver/test/test_rmt.c |  3 +++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/components/driver/rmt.c b/components/driver/rmt.c
index 9083deb299..41be13331c 100644
--- a/components/driver/rmt.c
+++ b/components/driver/rmt.c
@@ -63,7 +63,7 @@ typedef struct {
     portMUX_TYPE rmt_spinlock; // Mutex lock for protecting concurrent register/unregister of RMT channels' ISR
     rmt_isr_handle_t rmt_driver_intr_handle;
     rmt_tx_end_callback_t rmt_tx_end_callback;// Event called when transmission is ended
-    uint8_t rmt_driver_channels; // Bitmask of installed drivers' channels
+    uint8_t rmt_driver_channels; // Bitmask of installed drivers' channels, used to protect concurrent register/unregister of RMT channels' ISR
     bool rmt_module_enabled;
     uint32_t synchro_channel_mask; // Bitmap of channels already added in the synchronous group
 } rmt_contex_t;
@@ -951,7 +951,7 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)
 {
     esp_err_t err = ESP_OK;
     ESP_RETURN_ON_FALSE(channel < RMT_CHANNEL_MAX, ESP_ERR_INVALID_ARG, TAG, RMT_CHANNEL_ERROR_STR);
-    ESP_RETURN_ON_FALSE(rmt_contex.rmt_driver_channels & BIT(channel), ESP_ERR_INVALID_STATE, TAG, "No RMT driver for this channel");
+    // we allow to call this uninstall function on the same channel for multiple times
     if (p_rmt_obj[channel] == NULL) {
         return ESP_OK;
     }
@@ -977,7 +977,7 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)
 
     _lock_acquire_recursive(&(rmt_contex.rmt_driver_isr_lock));
     rmt_contex.rmt_driver_channels &= ~BIT(channel);
-    if (rmt_contex.rmt_driver_channels == 0) {
+    if (rmt_contex.rmt_driver_channels == 0 && rmt_contex.rmt_driver_intr_handle) {
         rmt_module_disable();
         // all channels have driver disabled
         err = rmt_isr_deregister(rmt_contex.rmt_driver_intr_handle);
@@ -985,10 +985,6 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)
     }
     _lock_release_recursive(&(rmt_contex.rmt_driver_isr_lock));
 
-    if (err != ESP_OK) {
-        return err;
-    }
-
     if (p_rmt_obj[channel]->tx_sem) {
         vSemaphoreDelete(p_rmt_obj[channel]->tx_sem);
         p_rmt_obj[channel]->tx_sem = NULL;
@@ -1014,13 +1010,12 @@ esp_err_t rmt_driver_uninstall(rmt_channel_t channel)
 
     free(p_rmt_obj[channel]);
     p_rmt_obj[channel] = NULL;
-    return ESP_OK;
+    return err;
 }
 
 esp_err_t rmt_driver_install(rmt_channel_t channel, size_t rx_buf_size, int intr_alloc_flags)
 {
     ESP_RETURN_ON_FALSE(channel < RMT_CHANNEL_MAX, ESP_ERR_INVALID_ARG, TAG, RMT_CHANNEL_ERROR_STR);
-    ESP_RETURN_ON_FALSE((rmt_contex.rmt_driver_channels & BIT(channel)) == 0, ESP_ERR_INVALID_STATE, TAG, "RMT driver already installed for channel");
 
     esp_err_t err = ESP_OK;
 
diff --git a/components/driver/test/test_rmt.c b/components/driver/test/test_rmt.c
index 1827242ad3..e2fc00e9e1 100644
--- a/components/driver/test/test_rmt.c
+++ b/components/driver/test/test_rmt.c
@@ -211,11 +211,14 @@ TEST_CASE("RMT multiple channels", "[rmt]")
 TEST_CASE("RMT install/uninstall test", "[rmt]")
 {
     rmt_config_t tx_cfg = RMT_DEFAULT_CONFIG_TX(RMT_DATA_IO, RMT_TX_CHANNEL_ENCODING_END);
+    // uninstall function is allowed to be called at any time
+    TEST_ESP_OK(rmt_driver_uninstall(tx_cfg.channel));
     TEST_ESP_OK(rmt_config(&tx_cfg));
     for (int i = 0; i < 100; i++) {
         TEST_ESP_OK(rmt_driver_install(tx_cfg.channel, 1000, 0));
         TEST_ESP_OK(rmt_driver_uninstall(tx_cfg.channel));
     }
+    TEST_ESP_OK(rmt_driver_uninstall(tx_cfg.channel));
     rmt_config_t rx_cfg = RMT_DEFAULT_CONFIG_RX(RMT_DATA_IO, RMT_RX_CHANNEL_ENCODING_START);
     TEST_ESP_OK(rmt_config(&rx_cfg));
     for (int i = 0; i < 100; i++) {