Skip to content

Conversation

@bugadani
Copy link
Contributor

@bugadani bugadani commented Jan 7, 2026

Fixes #4718

process_update only handles data in block sized chunks. After processing a message, it copies back the "remaining" data into the item's buffer. Previously, the function failed to update the number of buffered bytes in the buffer, essentially losing parts of the message. This PR fixes this issue.

@bugadani bugadani marked this pull request as ready for review January 7, 2026 11:05
Copilot AI review requested due to automatic review settings January 7, 2026 11:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical bug in the SHA accelerator where hash calculations produced incorrect results for certain input sizes. The root cause was that the process_update function failed to update the buffered_bytes field after copying unprocessed bytes back to the buffer, causing parts of messages to be lost in subsequent operations.

  • Fixed the missing buffered_bytes update in process_update function
  • Refactored duplicate state restoration code into a dedicated restore_state function
  • Enhanced test coverage to include non-block-aligned buffer sizes (264 bytes)

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
esp-hal/src/sha.rs Fixed the critical buffered_bytes tracking bug in process_update, refactored state restoration into a helper function, and added debug logging
hil-test/src/bin/ecc_rsa_sha.rs Enhanced test to include non-block-aligned size (264 bytes) to verify the fix works for partial blocks
esp-hal/CHANGELOG.md Added entry documenting the SHA hash calculation fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bugadani bugadani enabled auto-merge January 7, 2026 11:26
@bugadani bugadani added this pull request to the merge queue Jan 7, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 7, 2026
@MabezDev MabezDev added this pull request to the merge queue Jan 7, 2026
Merged via the queue into esp-rs:main with commit 52992a3 Jan 7, 2026
24 checks passed
@bugadani bugadani deleted the sha-debug branch January 7, 2026 12:06
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.

Sha digests: incorrect hashes

2 participants