Skip to content

Big Fix: Array bounds crash in key handling#113

Merged
KlausMu merged 2 commits into
OMOTE-Community:mainfrom
Stuckya:fix/keys-array-bounds-crash
Sep 27, 2025
Merged

Big Fix: Array bounds crash in key handling#113
KlausMu merged 2 commits into
OMOTE-Community:mainfrom
Stuckya:fix/keys-array-bounds-crash

Conversation

@Stuckya
Copy link
Copy Markdown
Contributor

@Stuckya Stuckya commented Sep 20, 2025

I was encountering intermittent crashes when my kids were using the remote. This led me to debugging the crashes and I uncovered 3 issues along the way. This was the 1st issue.

Problem:

LoadProhibited crash with EXCVADDR: 0x00000009 when pressing KEY_CHUP repeatedly, caused by array bounds violations in key handling code.

Root Cause:

  • Incorrect array indexing in doShortPress() : lastTimeSent[keyCode/keypadROWS][keyCode%keypadROWS].
  • Wrong keyCode calculation in keypad_processKeyStates(): row * keypadROWS + col instead of row * keypadCOLS + col.

Proposed Fix:

  1. Corrected array indexing to use proper row/col derivation.
  2. Fixed keyCode calculation.
  3. Added bounds checking with helper functions to improve readability isKeyPressInBounds() and isKeyPressRateLimited().
  4. Refactored nested conditionals with early returns, defensive programming.

@KlausMu
Copy link
Copy Markdown
Member

KlausMu commented Sep 20, 2025

Even if keypadROWS and keypadCOLS are mixed up at some place, did this really lead to a crash? Both keypadROWS and keypadCOLS are set to 5.

@Stuckya
Copy link
Copy Markdown
Contributor Author

Stuckya commented Sep 22, 2025

Even if keypadROWS and keypadCOLS are mixed up at some place, did this really lead to a crash? Both keypadROWS and keypadCOLS are set to 5.

Good point and I think you are correct, since both keypadROWS and keypadCOLS are 5, the math would work out the same either way, even if they got mixed up.

Regardless, I think this cleans the code up and prevents future regressions.

I suspect #114 was the real fix given EXCVADDR: 0x00000009 was the cause when debugging. I applied both fixes at the same time on my branch and only separated them for PR purposes. I can test these fixes independently later today.

@Stuckya
Copy link
Copy Markdown
Contributor Author

Stuckya commented Sep 24, 2025

Confirmed, #114 was the real fix and this should just be considered cleanup (improved readability) & hardening.

@KlausMu KlausMu merged commit ad350e1 into OMOTE-Community:main Sep 27, 2025
4 of 5 checks passed
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