Repository navigation
Conversation
OLD_TaskDeviceID only has N_TASKS - 9 elements, but clearTask() still wrote OLD_TaskDeviceID[task]. For task index 3 and up this writes past the array into TaskDevicePin1..3/TaskDevicePort of other tasks. The array is unused, so the write is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
|
Good find! |
Contributor
|
I already took care of this in #5522 but that hasn't been merged yet. It was kind of a 'hidden' fix 😅 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OLD_TaskDeviceIDwas shrunk toN_TASKS - 9elements when its storage was reused for the I2C2/I2C3 settings, butSettingsStruct::clearTask()still doesOLD_TaskDeviceID[task] = 0u. For task index 3 and up this writes 4 bytes past the array intoTaskDevicePin1..3/TaskDevicePortof other tasks.clearTask()runs when a task is deleted on the Devices page, when its plugin is changed, and onTaskClear.On ESP8266 (12 tasks) the damage shows up right away. Example:
TaskClear,5on a unit with a PZEM-004T on task 6 (SW serial RX GPIO1, TX GPIO3) and a switch on task 7 (GPIO16). After the clear,TaskDevicePin1of tasks 6, 7 and 8 was 0. Nothing changes until the settings are reloaded: after the next reboot the PZEM read on GPIO0 and returned NaN, and the switch read GPIO0 (always high). Downgrading the firmware did not help, because the bad values were already inconfig.dat. On ESP32 (32 tasks) the same happens for task index 23 and up.The array is unused (its comment already says so), so this PR removes the write.
Validation:
N_TASKS = 12): before the fix,clearTask(t)changes pins of other tasks for every t >= 3 (31 bytes in total), and-fsanitize=boundsreportsindex 3 out of bounds for type 'unsigned int [3]'. After the fix, no foreign pin bytes change.normal_ESP8266_4M1M(Arduino core 3.1.2) with this change.TaskClearon task 7:TaskDevicePin1..3/TaskDevicePortof all other tasks inconfig.datstayed the same.