From b3d80b2e5d4a9e19f335366aa777189aba92d4ae Mon Sep 17 00:00:00 2001 From: Chen Chen Date: Wed, 19 Nov 2025 11:59:51 +0800 Subject: [PATCH] fix(ledc): fix potential null dereference issue & add test case --- components/esp_driver_ledc/src/ledc.c | 11 +++++- .../test_apps/ledc/main/test_ledc.c | 39 +++++++++++++++++++ components/hal/include/hal/ledc_hal.h | 19 +++++---- components/hal/ledc_hal_iram.c | 5 --- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/components/esp_driver_ledc/src/ledc.c b/components/esp_driver_ledc/src/ledc.c index 2143982537..b10830b915 100644 --- a/components/esp_driver_ledc/src/ledc.c +++ b/components/esp_driver_ledc/src/ledc.c @@ -1535,11 +1535,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, diff --git a/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c b/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c index ebbbc982f3..5c5952be24 100644 --- a/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c +++ b/components/esp_driver_ledc/test_apps/ledc/main/test_ledc.c @@ -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; diff --git a/components/hal/include/hal/ledc_hal.h b/components/hal/include/hal/ledc_hal.h index 04d9ba0f9f..c8cafc8449 100644 --- a/components/hal/include/hal/ledc_hal.h +++ b/components/hal/include/hal/ledc_hal.h @@ -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 diff --git a/components/hal/ledc_hal_iram.c b/components/hal/ledc_hal_iram.c index 3a6279982c..804473132c 100644 --- a/components/hal/ledc_hal_iram.c +++ b/components/hal/ledc_hal_iram.c @@ -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); -}