Skip to content
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

Fix: Timeout mishandling issue #1812

Merged
merged 2 commits into from
May 4, 2024
Merged

Fix: Timeout mishandling issue #1812

merged 2 commits into from
May 4, 2024

Conversation

dragonmux
Copy link
Member

Detailed description

This PR addresses an issue in the platform timeouts subsystem where the firmware will execute and handle timeouts very wrongly for a few seconds after ~49.7 days of uptime.

The gist of the bug is that as the time_ms counter approaches UINT32_MAX and platform_timeout_set() starts generating expiration times close to 0, the comparison for whether the timeout has expired will incorrectly determine that it has at the start of the timeout validity interval. This is incredibly wrong and will cause all manner of broken behaviour in anything that depends on timeouts to function properly.

We fix this by detecting this edge condition where the uptime counter has the MSb set and the expiration time does not and force the comparison to false. This is wrong if a timeout is requested that is exceedingly large (~231ms) but as the firmware never does that and that would be a ~25.8 day timeout, this edge-case is irrelevant.

Your checklist for this pull request

Closing issues

@dragonmux dragonmux requested a review from esden April 29, 2024 19:33
@dragonmux dragonmux added Bug Confirmed bug BMP Firmware Black Magic Probe Firmware (not PC hosted software) labels Apr 29, 2024
@dragonmux dragonmux added this to the v2.0 release milestone Apr 29, 2024
@dragonmux dragonmux force-pushed the fix/timeout-mishandling-issue branch from f9e99a8 to eef7e8a Compare May 3, 2024 21:52
Copy link
Member

@esden esden left a comment

Choose a reason for hiding this comment

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

LGTM

@esden esden merged commit eef7e8a into main May 4, 2024
24 checks passed
@dragonmux dragonmux deleted the fix/timeout-mishandling-issue branch May 4, 2024 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BMP Firmware Black Magic Probe Firmware (not PC hosted software) Bug Confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants