Skip to content

Conversation

kaspar030
Copy link
Contributor

Backport of #16374

Contribution description

This PR fixes a rare race-condition in xtimer_msg_receive_timeout which can lead to corruption of the timer list and subsequent hard faults.
The race condition is triggered when:

  1. A message is sent to a thread using msg_send.
  2. The xtimer which sents the timeout message expires and executes before xtimer_remove(t) is called. This will cause the message queue for the thread to contain first a real message and second the timeout message. The timer will not be queued anymore, but the timeout message will still be in the queue.
  3. The xtimer_msg_receive_timeout function is called again. This will queue a new xtimer while the timeout message of the previous timer is still in the buffer. _msg_wait will see the old timeout message, think that the current xtimer has already expired and will not remove the timer. When xtimer_msg_receive_timeout returns, the timer will still be queued. However, as it is allocated on the stack it is no longer valid. This causes the timer_list_head to now point to invalid memory. Crashes ensue.

Testing procedure

This bug was found, validated, and fixed using a proprietary application. I have not written a separate example application which exhibits the problem which I could publish.

@kaspar030 kaspar030 added Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs >1 ACK Integration Process: This PR requires more than one ACK Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Apr 22, 2021
@kaspar030 kaspar030 removed the Process: needs >1 ACK Integration Process: This PR requires more than one ACK label Apr 22, 2021
@kaspar030 kaspar030 force-pushed the backport/2021.04/master branch from 3f65b75 to 9bb21e2 Compare April 22, 2021 20:01
@kaspar030 kaspar030 force-pushed the backport/2021.04/master branch from 9bb21e2 to 30f4486 Compare April 26, 2021 08:07
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

@kaspar030 kaspar030 added this to the Release 2021.04 milestone Apr 26, 2021
@kaspar030 kaspar030 merged commit dc8e56d into RIOT-OS:2021.04-branch Apr 26, 2021
@kaspar030 kaspar030 deleted the backport/2021.04/master branch April 26, 2021 09:44
@kaspar030 kaspar030 added the Release notes: ignored Set on PRs that have been considered for inclusion in the current release's notes but were minor. label Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Release notes: ignored Set on PRs that have been considered for inclusion in the current release's notes but were minor. Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants