Skip to content

fix(cam_hal): guard cam_verify_jpeg_eoi() against buffer-underflow #759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RubenKelevra
Copy link

@RubenKelevra RubenKelevra commented Jul 1, 2025

Description

If DMA returns a frame shorter than two bytes, the previous code did:

dptr = inbuf + length - 2;

which under-flows the pointer and produces undefined behaviour.

Behaviour for valid frames (length ≥ 2) is unchanged; damaged or empty buffers are now discarded safely.

Related

In the search for the cause of crashes reported by @turenkomv here: esphome/esphome#8832 (comment) I found this underflow of a pointer, which can cause undefined behaviour.

Testing

@turenkomv would you be so kind and would test this on your hardware? Tag is fixes_for_m5_0.2, which is the currently used version in ESPHome with both patches on top.

Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass. (?)
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

If DMA returns a frame shorter than two bytes, the previous code
did:

    dptr = inbuf + length - 2;

which under-flows the pointer and produces undefined behaviour.

Behaviour for valid frames (length ≥ 2) is unchanged; damaged or
empty buffers are now discarded safely.
@turenkomv
Copy link

No effect, still getting the same error.

Just to double-check — am I only testing the changes from this PR, and not these ones #758 ?

@RubenKelevra
Copy link
Author

Just to double-check — am I only testing the changes from this PR, and not these ones #758 ?

No, please both changes. The tag fixes_for_m5_0.2 I've prepared for your contains both fixes on top of 2.0.15, which is the version used in ESPHome.

@turenkomv
Copy link

Tried fixes_for_m5_0.2 — no changes, same behavior

@RubenKelevra
Copy link
Author

Yeah, I found the next bug.

Grab a coffee, this may take a while :D

@turenkomv
Copy link

Enabled the flags:

CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK: "y"
CONFIG_FREERTOS_CHECK_STACKOVERFLOW: "2"

Here's the output from xtensa-esp32s3-elf-addr2line — hopefully this helps:

C:\Projects\xtensa-esp32s3-elf-gcc8_4_0-esp-2021r2-win32\xtensa-esp32s3-elf\bin>xtensa-esp32s3-elf-addr2line.exe -pfiaC -e "C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1\.pioenvs\m5stack-camera-1\firmware.elf" 0x4037ef47 0x420484af 0x420485d5 0x4200f951 0x42016a3e 0x420a4a49 0x403845b2 0x4201a72a 0x42019cca 0x4200bbfa
0x4037ef47: xPortSetInterruptMaskFromISR at C:\Users\turen\.platformio\packages\framework-espidf\components\freertos\FreeRTOS-Kernel\portable\xtensa\include\freertos/portmacro.h:552
 (inlined by) xPortEnterCriticalTimeout at C:\Users\turen\.platformio\packages\framework-espidf\components\freertos\FreeRTOS-Kernel\portable\xtensa/port.c:478
0x420484af: vPortEnterCritical at C:\Users\turen\.platformio\packages\framework-espidf\components\freertos\FreeRTOS-Kernel\portable\xtensa\include\freertos/portmacro.h:567
 (inlined by) find_key at C:\Users\turen\.platformio\packages\framework-espidf\components\pthread/pthread_local_storage.c:77
0x420485d5: pthread_setspecific at C:\Users\turen\.platformio\packages\framework-espidf\components\pthread/pthread_local_storage.c:199
0x4200f951: esphome::logger::Logger::check_and_set_task_log_recursion_(bool) at C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1/src/esphome/components/logger/logger.h:298
 (inlined by) esphome::logger::Logger::check_and_set_task_log_recursion_(bool) at C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1/src/esphome/components/logger/logger.h:287
 (inlined by) esphome::logger::Logger::log_vprintf_(unsigned char, char const*, int, char const*, __va_list_tag) at C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1/src/esphome/components/logger/logger.cpp:35
0x42016a3e: esphome::esp_idf_log_vprintf_(char const*, __va_list_tag) at C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1/src/esphome/core/log.cpp:56
0x420a4a49: esp_log_writev at C:\Users\turen\.platformio\packages\framework-espidf\components\log/log.c:210
0x403845b2: esp_log_write at C:\Users\turen\.platformio\packages\framework-espidf\components\log/log.c:220
0x4201a72a: cam_take at C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1/managed_components/espressif__esp32-camera/driver/cam_hal.c:494 (discriminator 1)
0x42019cca: esp_camera_fb_get at C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1/managed_components/espressif__esp32-camera/driver/esp_camera.c:372
0x4200bbfa: esphome::esp32_camera::ESP32Camera::framebuffer_task(void*) at C:\Projects\m5stack-camera-1\.esphome\build\m5stack-camera-1/src/esphome/components/esp32_camera/esp32_camera.cpp:412

@RubenKelevra
Copy link
Author

Yes, that's very helpful.

@RubenKelevra
Copy link
Author

I found the culprit. I'll provide another fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants