firmware: fix race conditions in control out transfers (-3 bytes XRAM).#703
Merged
whitequark merged 1 commit intoGlasgowEmbedded:mainfrom Oct 8, 2025
Conversation
purdeaandrei
force-pushed
the
f_fix_control_out_transfer_race_conditions
branch
from
a628e16 to
b1e7539
Compare
purdeaandrei
changed the title
purdeaandrei
force-pushed
the
f_fix_control_out_transfer_race_conditions
branch
from
b1e7539 to
cf672e2
Compare
whitequark
approved these changes
Oct 6, 2025
Member
|
I think you might need to update the submodule. |
Contributor
Author
Yes, but PR18 in libfx2 isn't merged yet, only approved. I'll do that as soon as it's merged. |
purdeaandrei
force-pushed
the
f_fix_control_out_transfer_race_conditions
branch
from
cf672e2 to
8591432
Compare
purdeaandrei
changed the title
purdeaandrei
force-pushed
the
f_fix_control_out_transfer_race_conditions
branch
from
8591432 to
1091d3d
Compare
Contributor
Author
|
Alright,
Should be ready to merge. |
Member
|
Thanks! |
Member
|
(I think deploying the firmware may have to happen in a separate PR.) |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
purdeaandrei
force-pushed
the
f_fix_control_out_transfer_race_conditions
branch
from
1091d3d to
be634a5
Compare
Contributor
Author
|
Okay, re-pushed without the deploy |
Before this change, the EP0BUF buffer used by control out transfers could be overwritten by new control transfers before the firmware is finished processing the previous control transfer. The easiest way to illustrate this problem would be to run in a different terminal the following: ```bash while true; do lsusb -v -d 20b7:9db1 > /dev/null; done ``` While this is running, glasglow is completely unusable. Presumably even a single lsusb run could cause corruption, if it happens to be issued at the wrong time. With this change glasgow is now usable, even if the above loop is running. Please see GlasgowEmbedded/libfx2#18 for more details.
purdeaandrei
force-pushed
the
f_fix_control_out_transfer_race_conditions
branch
from
be634a5 to
7bbbd25
Compare
Contributor
Author
|
Aand pushed again, rebased |
Member
|
(You have write access now, so I think it automatically replaces your PR in the queue if you push to it, or something like that?) |
Contributor
Author
Not sure, I definitely didn't add it to the queue |
Member
|
I did. But if you don't have write access, then it kicks you out of the queue after a (force-/normal-)push. |
purdeaandrei
added a commit
to purdeaandrei/glasgow
that referenced
this pull request
Nov 1, 2025
…tes XRAM) These were missed by PR GlasgowEmbedded#703, because they used `SETUP_EP0_BUF(2)` with a non-zero argument for Control OUT transfers. Even though these didn't follow libfx2's (old) rule, of setting length to 0 when using Control OUT transfers, they did mostly work, because the FX2 actually ignores the length on OUT transfers. PR GlasgowEmbedded#703 assumed that all non-zero `SETUP_EP0_BUF` calls were IN transfer, which was wrong, so these instances remained vulnerable to the race condition. This change fixes those too.
purdeaandrei
added a commit
to purdeaandrei/glasgow
that referenced
this pull request
Nov 1, 2025
…tes XRAM). These were missed by PR GlasgowEmbedded#703, because they used `SETUP_EP0_BUF(2)` with a non-zero argument for Control OUT transfers. Even though these didn't follow libfx2's (old) rule, of setting length to 0 when using Control OUT transfers, they did mostly work, because the FX2 actually ignores the length on OUT transfers. PR GlasgowEmbedded#703 assumed that all non-zero `SETUP_EP0_BUF` calls were IN transfer, which was wrong, so these instances remained vulnerable to the race condition. This change fixes those too.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Nov 2, 2025
These were missed by PR #703, because they used `SETUP_EP0_BUF(2)` with a non-zero argument for Control OUT transfers. Even though these didn't follow libfx2's (old) rule, of setting length to 0 when using Control OUT transfers, they did mostly work, because the FX2 actually ignores the length on OUT transfers. PR #703 assumed that all non-zero `SETUP_EP0_BUF` calls were IN transfer, which was wrong, so these instances remained vulnerable to the race condition. This change fixes those too.
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
2 participants
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.
This PR depends on GlasgowEmbedded/libfx2#18
This PR is ready for review but it should only be merged after the libfx2 PR is merged,
and then this PR should also include an update to the submodule reference.
Before this change, the EP0BUF buffer used by control out transfers could be overwritten by new control transfers before the firmware is finished processing the previous control transfer.
The easiest way to illustrate this problem would be to run in a different terminal the following:
While this is running, glasglow is completely unusable. Presumably even a single lsusb run could cause corruption, if it happens to be issued at the wrong time.
With this change glasgow is now usable, even if the above loop is running.