gdma: fix potential use after free issue

This commit is contained in:
morris 2021-12-14 10:08:26 +08:00
parent 1d0ae38c11
commit 14c926350a
4 changed files with 51 additions and 56 deletions

View File

@ -21,8 +21,11 @@ set(srcs
set(includes "include" "${target}/include" "deprecated") set(includes "include" "${target}/include" "deprecated")
if(CONFIG_SOC_MCPWM_SUPPORTED) if(CONFIG_SOC_MCPWM_SUPPORTED)
list(APPEND srcs list(APPEND srcs "mcpwm.c")
"mcpwm.c") endif()
if(CONFIG_SOC_GDMA_SUPPORTED)
list(APPEND srcs "gdma.c")
endif() endif()
if(${target} STREQUAL "esp32") if(${target} STREQUAL "esp32")
@ -62,7 +65,6 @@ endif()
if(${target} STREQUAL "esp32s3") if(${target} STREQUAL "esp32s3")
list(APPEND srcs "adc_common.c" list(APPEND srcs "adc_common.c"
"dedic_gpio.c" "dedic_gpio.c"
"gdma.c"
"sdmmc_host.c" "sdmmc_host.c"
"sdmmc_transaction.c" "sdmmc_transaction.c"
"rmt.c" "rmt.c"
@ -76,8 +78,7 @@ if(${target} STREQUAL "esp32s3")
endif() endif()
if(IDF_TARGET STREQUAL "esp32c3") if(IDF_TARGET STREQUAL "esp32c3")
list(APPEND srcs "gdma.c" list(APPEND srcs "spi_slave_hd.c"
"spi_slave_hd.c"
"adc_common.c" "adc_common.c"
"dedic_gpio.c" "dedic_gpio.c"
"usb_serial_jtag.c" "usb_serial_jtag.c"
@ -91,8 +92,7 @@ if(IDF_TARGET STREQUAL "esp32c3")
endif() endif()
if(IDF_TARGET STREQUAL "esp32h2") if(IDF_TARGET STREQUAL "esp32h2")
list(APPEND srcs "gdma.c" list(APPEND srcs "spi_slave_hd.c"
"spi_slave_hd.c"
"adc_common.c" "adc_common.c"
"dedic_gpio.c" "dedic_gpio.c"
"i2s.c" "i2s.c"

View File

@ -109,11 +109,9 @@ struct gdma_rx_channel_t {
}; };
static gdma_group_t *gdma_acquire_group_handle(int group_id); static gdma_group_t *gdma_acquire_group_handle(int group_id);
static void gdma_release_group_handle(gdma_group_t *group);
static gdma_pair_t *gdma_acquire_pair_handle(gdma_group_t *group, int pair_id); static gdma_pair_t *gdma_acquire_pair_handle(gdma_group_t *group, int pair_id);
static void gdma_release_group_handle(gdma_group_t *group);
static void gdma_release_pair_handle(gdma_pair_t *pair); static void gdma_release_pair_handle(gdma_pair_t *pair);
static void gdma_uninstall_group(gdma_group_t *group);
static void gdma_uninstall_pair(gdma_pair_t *pair);
static esp_err_t gdma_del_tx_channel(gdma_channel_t *dma_channel); static esp_err_t gdma_del_tx_channel(gdma_channel_t *dma_channel);
static esp_err_t gdma_del_rx_channel(gdma_channel_t *dma_channel); static esp_err_t gdma_del_rx_channel(gdma_channel_t *dma_channel);
static esp_err_t gdma_install_rx_interrupt(gdma_rx_channel_t *rx_chan); static esp_err_t gdma_install_rx_interrupt(gdma_rx_channel_t *rx_chan);
@ -161,27 +159,28 @@ esp_err_t gdma_new_channel(const gdma_channel_alloc_config_t *config, gdma_chann
for (int i = 0; i < SOC_GDMA_GROUPS && search_code; i++) { // loop to search group for (int i = 0; i < SOC_GDMA_GROUPS && search_code; i++) { // loop to search group
group = gdma_acquire_group_handle(i); group = gdma_acquire_group_handle(i);
for (int j = 0; j < SOC_GDMA_PAIRS_PER_GROUP && search_code && group; j++) { // loop to search pair ESP_GOTO_ON_FALSE(group, ESP_ERR_NO_MEM, err, TAG, "no mem for group(%d)", i);
for (int j = 0; j < SOC_GDMA_PAIRS_PER_GROUP && search_code; j++) { // loop to search pair
pair = gdma_acquire_pair_handle(group, j); pair = gdma_acquire_pair_handle(group, j);
if (pair) { ESP_GOTO_ON_FALSE(pair, ESP_ERR_NO_MEM, err, TAG, "no mem for pair(%d,%d)", i, j);
portENTER_CRITICAL(&pair->spinlock); portENTER_CRITICAL(&pair->spinlock);
if (!(search_code & pair->occupy_code)) { // pair has suitable position for acquired channel(s) if (!(search_code & pair->occupy_code)) { // pair has suitable position for acquired channel(s)
pair->occupy_code |= search_code; pair->occupy_code |= search_code;
search_code = 0; // exit search loop search_code = 0; // exit search loop
} }
portEXIT_CRITICAL(&pair->spinlock); portEXIT_CRITICAL(&pair->spinlock);
if (!search_code) { if (search_code) {
portENTER_CRITICAL(&group->spinlock);
group->pair_ref_counts[j]++; // channel obtains a reference to pair
portEXIT_CRITICAL(&group->spinlock);
}
}
gdma_release_pair_handle(pair); gdma_release_pair_handle(pair);
pair = NULL;
}
} // loop used to search pair } // loop used to search pair
if (search_code) {
gdma_release_group_handle(group); gdma_release_group_handle(group);
group = NULL;
}
} // loop used to search group } // loop used to search group
ESP_GOTO_ON_FALSE(search_code == 0, ESP_ERR_NOT_FOUND, err, TAG, "no free gdma channel, search code=%d", search_code); ESP_GOTO_ON_FALSE(search_code == 0, ESP_ERR_NOT_FOUND, err, TAG, "no free gdma channel, search code=%d", search_code);
assert(pair && group); // pair and group handle shouldn't be NULL
search_done: search_done:
// register TX channel // register TX channel
if (alloc_tx_channel) { if (alloc_tx_channel) {
@ -214,6 +213,12 @@ err:
if (alloc_rx_channel) { if (alloc_rx_channel) {
free(alloc_rx_channel); free(alloc_rx_channel);
} }
if (pair) {
gdma_release_pair_handle(pair);
}
if (group) {
gdma_release_group_handle(group);
}
return ret; return ret;
} }
@ -525,7 +530,7 @@ err:
return ret; return ret;
} }
static void gdma_uninstall_group(gdma_group_t *group) static void gdma_release_group_handle(gdma_group_t *group)
{ {
int group_id = group->group_id; int group_id = group->group_id;
bool do_deinitialize = false; bool do_deinitialize = false;
@ -581,14 +586,7 @@ out:
return group; return group;
} }
static void gdma_release_group_handle(gdma_group_t *group) static void gdma_release_pair_handle(gdma_pair_t *pair)
{
if (group) {
gdma_uninstall_group(group);
}
}
static void gdma_uninstall_pair(gdma_pair_t *pair)
{ {
gdma_group_t *group = pair->group; gdma_group_t *group = pair->group;
int pair_id = pair->pair_id; int pair_id = pair->pair_id;
@ -606,8 +604,7 @@ static void gdma_uninstall_pair(gdma_pair_t *pair)
if (do_deinitialize) { if (do_deinitialize) {
free(pair); free(pair);
ESP_LOGD(TAG, "del pair (%d,%d)", group->group_id, pair_id); ESP_LOGD(TAG, "del pair (%d,%d)", group->group_id, pair_id);
gdma_release_group_handle(group);
gdma_uninstall_group(group);
} }
} }
@ -646,17 +643,12 @@ out:
return pair; return pair;
} }
static void gdma_release_pair_handle(gdma_pair_t *pair)
{
if (pair) {
gdma_uninstall_pair(pair);
}
}
static esp_err_t gdma_del_tx_channel(gdma_channel_t *dma_channel) static esp_err_t gdma_del_tx_channel(gdma_channel_t *dma_channel)
{ {
gdma_pair_t *pair = dma_channel->pair; gdma_pair_t *pair = dma_channel->pair;
gdma_group_t *group = pair->group; gdma_group_t *group = pair->group;
int pair_id = pair->pair_id;
int group_id = group->group_id;
gdma_tx_channel_t *tx_chan = __containerof(dma_channel, gdma_tx_channel_t, base); gdma_tx_channel_t *tx_chan = __containerof(dma_channel, gdma_tx_channel_t, base);
portENTER_CRITICAL(&pair->spinlock); portENTER_CRITICAL(&pair->spinlock);
pair->tx_chan = NULL; pair->tx_chan = NULL;
@ -666,15 +658,16 @@ static esp_err_t gdma_del_tx_channel(gdma_channel_t *dma_channel)
if (dma_channel->intr) { if (dma_channel->intr) {
esp_intr_free(dma_channel->intr); esp_intr_free(dma_channel->intr);
portENTER_CRITICAL(&pair->spinlock); portENTER_CRITICAL(&pair->spinlock);
gdma_ll_tx_enable_interrupt(group->hal.dev, pair->pair_id, UINT32_MAX, false); // disable all interupt events gdma_ll_tx_enable_interrupt(group->hal.dev, pair_id, UINT32_MAX, false); // disable all interupt events
gdma_ll_tx_clear_interrupt_status(group->hal.dev, pair->pair_id, UINT32_MAX); // clear all pending events gdma_ll_tx_clear_interrupt_status(group->hal.dev, pair_id, UINT32_MAX); // clear all pending events
portEXIT_CRITICAL(&pair->spinlock); portEXIT_CRITICAL(&pair->spinlock);
ESP_LOGD(TAG, "uninstall interrupt service for tx channel (%d,%d)", group->group_id, pair->pair_id); ESP_LOGD(TAG, "uninstall interrupt service for tx channel (%d,%d)", group_id, pair_id);
} }
ESP_LOGD(TAG, "del tx channel (%d,%d)", group->group_id, pair->pair_id);
free(tx_chan); free(tx_chan);
gdma_uninstall_pair(pair); ESP_LOGD(TAG, "del tx channel (%d,%d)", group_id, pair_id);
// channel has a reference on pair, release it now
gdma_release_pair_handle(pair);
return ESP_OK; return ESP_OK;
} }
@ -682,6 +675,8 @@ static esp_err_t gdma_del_rx_channel(gdma_channel_t *dma_channel)
{ {
gdma_pair_t *pair = dma_channel->pair; gdma_pair_t *pair = dma_channel->pair;
gdma_group_t *group = pair->group; gdma_group_t *group = pair->group;
int pair_id = pair->pair_id;
int group_id = group->group_id;
gdma_rx_channel_t *rx_chan = __containerof(dma_channel, gdma_rx_channel_t, base); gdma_rx_channel_t *rx_chan = __containerof(dma_channel, gdma_rx_channel_t, base);
portENTER_CRITICAL(&pair->spinlock); portENTER_CRITICAL(&pair->spinlock);
pair->rx_chan = NULL; pair->rx_chan = NULL;
@ -691,15 +686,15 @@ static esp_err_t gdma_del_rx_channel(gdma_channel_t *dma_channel)
if (dma_channel->intr) { if (dma_channel->intr) {
esp_intr_free(dma_channel->intr); esp_intr_free(dma_channel->intr);
portENTER_CRITICAL(&pair->spinlock); portENTER_CRITICAL(&pair->spinlock);
gdma_ll_rx_enable_interrupt(group->hal.dev, pair->pair_id, UINT32_MAX, false); // disable all interupt events gdma_ll_rx_enable_interrupt(group->hal.dev, pair_id, UINT32_MAX, false); // disable all interupt events
gdma_ll_rx_clear_interrupt_status(group->hal.dev, pair->pair_id, UINT32_MAX); // clear all pending events gdma_ll_rx_clear_interrupt_status(group->hal.dev, pair_id, UINT32_MAX); // clear all pending events
portEXIT_CRITICAL(&pair->spinlock); portEXIT_CRITICAL(&pair->spinlock);
ESP_LOGD(TAG, "uninstall interrupt service for rx channel (%d,%d)", group->group_id, pair->pair_id); ESP_LOGD(TAG, "uninstall interrupt service for rx channel (%d,%d)", group_id, pair_id);
} }
ESP_LOGD(TAG, "del rx channel (%d,%d)", group->group_id, pair->pair_id);
free(rx_chan); free(rx_chan);
gdma_uninstall_pair(pair); ESP_LOGD(TAG, "del rx channel (%d,%d)", group_id, pair_id);
gdma_release_pair_handle(pair);
return ESP_OK; return ESP_OK;
} }

View File

@ -145,7 +145,7 @@ config SOC_GDMA_GROUPS
config SOC_GDMA_PAIRS_PER_GROUP config SOC_GDMA_PAIRS_PER_GROUP
int int
default 3 default 1
config SOC_GDMA_TX_RX_SHARE_INTERRUPT config SOC_GDMA_TX_RX_SHARE_INTERRUPT
bool bool

View File

@ -74,7 +74,7 @@
/*-------------------------- GDMA CAPS -------------------------------------*/ /*-------------------------- GDMA CAPS -------------------------------------*/
#define SOC_GDMA_GROUPS (1U) // Number of GDMA groups #define SOC_GDMA_GROUPS (1U) // Number of GDMA groups
#define SOC_GDMA_PAIRS_PER_GROUP (3) // Number of GDMA pairs in each group #define SOC_GDMA_PAIRS_PER_GROUP (1U) // Number of GDMA pairs in each group
#define SOC_GDMA_TX_RX_SHARE_INTERRUPT (1) // TX and RX channel in the same pair will share the same interrupt source number #define SOC_GDMA_TX_RX_SHARE_INTERRUPT (1) // TX and RX channel in the same pair will share the same interrupt source number
/*-------------------------- GPIO CAPS ---------------------------------------*/ /*-------------------------- GPIO CAPS ---------------------------------------*/