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

BitArray race conditions may lead to memory corruption #30141

Closed
GrabYourPitchforks opened this issue Jul 4, 2019 · 4 comments · Fixed by dotnet/corefx#39270
Closed

BitArray race conditions may lead to memory corruption #30141

GrabYourPitchforks opened this issue Jul 4, 2019 · 4 comments · Fixed by dotnet/corefx#39270
Assignees
Milestone

Comments

@GrabYourPitchforks
Copy link
Member

As of dotnet/corefx#33781, the BitArray class contains unsafe code as a performance optimization. Since BitArray instances are not thread-safe, this opens the possibility that improper cross-thread access to a BitArray instance may result in global memory corruption.

We should modify the BitArray internal logic to be resilient against this possibility. We don't need to make the type thread-safe, but improper cross-thread access shouldn't be able to cause damage beyond corruption of the individual BitArray instances involved.

@benaadams
Copy link
Member

Work on local array references rather than class references?

@GrabYourPitchforks
Copy link
Member Author

@benaadams Yeah, that should work. I didn't want to be too prescriptive in the bug description. :)

@benaadams
Copy link
Member

Also probably faster too :)

@mikedn
Copy link
Contributor

mikedn commented Jul 5, 2019

Simply using local array references is not sufficient. You have to acquire the array and the length atomically but that's not really possible so the only solution is to validate the length once the 2 have been copied to local variables.

P.S. Though perhaps this isn't needed, it seems that the relevant operations use m_array's Length and not m_length so just copying the array to a local should be enough.

@stephentoub stephentoub self-assigned this Jul 7, 2019
@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 3.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants