fix(ledc): fix potential null dereference issue & add test case

This commit is contained in:
Chen Chen
2025-11-19 11:59:51 +08:00
parent 851ffae394
commit 698cbe884b
6 changed files with 66 additions and 20 deletions

View File

@@ -1533,11 +1533,20 @@ exit:
esp_err_t ledc_fade_func_install(int intr_alloc_flags)
{
LEDC_CHECK(s_ledc_fade_isr_handle == NULL, "fade function already installed", ESP_ERR_INVALID_STATE);
ledc_mode_t speed_mode = LEDC_SPEED_MODE_MAX;
for (int i = 0; i < LEDC_SPEED_MODE_MAX; i++) {
if (p_ledc_obj[i] != NULL) {
speed_mode = i;
break;
}
}
//OR intr_alloc_flags with ESP_INTR_FLAG_IRAM because the fade isr is in IRAM
return esp_intr_alloc_intrstatus(
ETS_LEDC_INTR_SOURCE,
intr_alloc_flags | ESP_INTR_FLAG_IRAM,
(uint32_t)ledc_hal_get_fade_end_intr_addr(&(p_ledc_obj[0]->ledc_hal)),
(uint32_t)ledc_hal_get_fade_end_intr_addr(&(p_ledc_obj[speed_mode]->ledc_hal)),
LEDC_LL_FADE_END_INTR_MASK,
ledc_fade_isr,
NULL,

View File

@@ -258,6 +258,45 @@ TEST_CASE("LEDC fade with step", "[ledc]")
ledc_fade_func_uninstall();
}
#if SOC_LEDC_SUPPORT_HS_MODE
TEST_CASE("LEDC fade install with low speed mode only", "[ledc]")
{
// This test verifies that ledc_fade_func_install works correctly when only
// LEDC_LOW_SPEED_MODE is initialized, without initializing LEDC_HIGH_SPEED_MODE.
// This tests the fix for NULL pointer dereference issue.
// Only initialize low speed mode, do NOT initialize high speed mode
ledc_channel_config_t ledc_ch_config = initialize_channel_config();
ledc_ch_config.speed_mode = LEDC_LOW_SPEED_MODE;
ledc_ch_config.duty = 0;
ledc_ch_config.channel = LEDC_CHANNEL_0;
ledc_ch_config.timer_sel = LEDC_TIMER_0;
ledc_timer_config_t ledc_time_config = create_default_timer_config();
ledc_time_config.speed_mode = LEDC_LOW_SPEED_MODE;
ledc_time_config.timer_num = LEDC_TIMER_0;
// Initialize only low speed mode
TEST_ESP_OK(ledc_channel_config(&ledc_ch_config));
TEST_ESP_OK(ledc_timer_config(&ledc_time_config));
vTaskDelay(5 / portTICK_PERIOD_MS);
TEST_ESP_OK(ledc_fade_func_install(0));
// Verify that fade functionality works correctly with low speed mode
TEST_ESP_OK(ledc_set_fade_with_time(LEDC_LOW_SPEED_MODE, LEDC_CHANNEL_0, 4000, 200));
TEST_ESP_OK(ledc_fade_start(LEDC_LOW_SPEED_MODE, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
TEST_ASSERT_EQUAL_INT32(4000, ledc_get_duty(LEDC_LOW_SPEED_MODE, LEDC_CHANNEL_0));
// Test fade down
TEST_ESP_OK(ledc_set_fade_with_time(LEDC_LOW_SPEED_MODE, LEDC_CHANNEL_0, 0, 200));
TEST_ESP_OK(ledc_fade_start(LEDC_LOW_SPEED_MODE, LEDC_CHANNEL_0, LEDC_FADE_WAIT_DONE));
TEST_ASSERT_EQUAL_INT32(0, ledc_get_duty(LEDC_LOW_SPEED_MODE, LEDC_CHANNEL_0));
// Cleanup
ledc_fade_func_uninstall();
}
#endif // SOC_LEDC_SUPPORT_HS_MODE
TEST_CASE("LEDC fast switching duty with fade_wait_done", "[ledc]")
{
const ledc_mode_t test_speed_mode = TEST_SPEED_MODE;

View File

@@ -14,7 +14,7 @@
#include "esp_err.h"
#include "driver/sdmmc_types.h"
#include "driver/sdmmc_default_configs.h"
#include "soc/gpio_num.h"
#include "driver/gpio.h"
#ifdef __cplusplus
extern "C" {

View File

@@ -396,14 +396,6 @@ void ledc_hal_get_fade_end_intr_status(ledc_hal_context_t *hal, uint32_t *intr_s
*/
void ledc_hal_clear_fade_end_intr_status(ledc_hal_context_t *hal, ledc_channel_t channel_num);
/**
* @brief Get the address of the fade end interrupt status register.
*
* @param hal Context of the HAL layer
* @return Pointer to the fade end interrupt status register.
*/
volatile void* ledc_hal_get_fade_end_intr_addr(ledc_hal_context_t *hal);
/**
* @brief Get clock config of LEDC timer
*
@@ -415,6 +407,17 @@ volatile void* ledc_hal_get_fade_end_intr_addr(ledc_hal_context_t *hal);
*/
void ledc_hal_get_clk_cfg(ledc_hal_context_t *hal, ledc_timer_t timer_sel, ledc_clk_cfg_t *clk_cfg);
/**
* @brief Get the address of the fade end interrupt status register.
*
* @param hal Context of the HAL layer
* @return Pointer to the fade end interrupt status register.
*/
static inline volatile void* ledc_hal_get_fade_end_intr_addr(ledc_hal_context_t *hal)
{
return ledc_ll_get_fade_end_intr_addr(hal->dev);
}
#endif //#if SOC_LEDC_SUPPORTED
#ifdef __cplusplus

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -153,7 +153,7 @@ extern "C" {
#endif
#if SOC_SDIO_SLAVE_SUPPORTED
/// Space used for each sending descriptor. Should initialize the sendbuf accoring to this size.
/// Space used for each sending descriptor. Should initialize the sendbuf according to this size.
#define SDIO_SLAVE_SEND_DESC_SIZE sizeof(sdio_slave_hal_send_desc_t)
@@ -239,7 +239,7 @@ typedef struct {
/**
* Initialize the HAL, should provide buffers to the context and configure the
* members before this funciton is called.
* members before this function is called.
*
* @param hal Context of the HAL layer.
*/
@@ -350,7 +350,7 @@ esp_err_t sdio_slave_hal_send_get_next_finished_arg(sdio_slave_context_t *hal, v
*
* @note Only call when the DMA is stopped!
* @param hal Context of the HAL layer.
* @param out_arg Argument indiciating the buffer to send
* @param out_arg Argument indicating the buffer to send
* @param out_return_cnt Space in the queue released after this descriptor is flushed.
* @return
* - ESP_ERR_INVALID_STATE: This function call be called only when the DMA is stopped.
@@ -476,7 +476,7 @@ void sdio_slave_hal_recv_flush_one_buffer(sdio_slave_context_t *hal);
/**
* Enable some of the interrupts for the host.
*
* @note May have concurrency issue wit the host or other tasks, suggest only use it during
* @note May have concurrency issue with the host or other tasks, suggest only use it during
* initialization.
* @param hal Context of the HAL layer.
* @param mask Bitwise mask for the interrupts to enable.

View File

@@ -75,8 +75,3 @@ void ledc_hal_clear_fade_end_intr_status(ledc_hal_context_t *hal, ledc_channel_t
{
ledc_ll_clear_fade_end_intr_status(hal->dev, hal->speed_mode, channel_num);
}
volatile void* ledc_hal_get_fade_end_intr_addr(ledc_hal_context_t *hal)
{
return ledc_ll_get_fade_end_intr_addr(hal->dev);
}