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 data race setting default_DSO_meth #16972

Closed

Conversation

paulidale
Copy link
Contributor

The global variable default_DSO_meth was potentially set multiple times by different threads. It turns out that it could only be set to a single value so the race is harmless but still better avoided. The fix here simply removes the global and accesses the value it was set to via the DSO_METHOD_openssl() call.

Problem discovered via #16970, but this does not resolve that issue because there are other concerns.

  • documentation is added or updated
  • tests are added or updated

The global variable `default_DSO_meth` was potentially set multiple times by
different threads.  It turns out that it could only be set to a single value
so the race is harmless but still better avoided.  The fix here simply removes
the global and accesses the value it was set to via the `DSO_METHOD_openssl()`
call.

Problem discovered via openssl#16970, but this does not resolve that issue because
there are other concerns.
@paulidale paulidale added branch: master Merge to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug branch: 3.0 Merge to openssl-3.0 branch labels Nov 5, 2021
@paulidale paulidale self-assigned this Nov 5, 2021
@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Nov 5, 2021
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Nov 6, 2021
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Nov 6, 2021
openssl-machine pushed a commit that referenced this pull request Nov 7, 2021
The global variable `default_DSO_meth` was potentially set multiple times by
different threads.  It turns out that it could only be set to a single value
so the race is harmless but still better avoided.  The fix here simply removes
the global and accesses the value it was set to via the `DSO_METHOD_openssl()`
call.

Problem discovered via #16970, but this does not resolve that issue because
there are other concerns.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #16972)
openssl-machine pushed a commit that referenced this pull request Nov 7, 2021
The global variable `default_DSO_meth` was potentially set multiple times by
different threads.  It turns out that it could only be set to a single value
so the race is harmless but still better avoided.  The fix here simply removes
the global and accesses the value it was set to via the `DSO_METHOD_openssl()`
call.

Problem discovered via #16970, but this does not resolve that issue because
there are other concerns.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #16972)

(cherry picked from commit e6a10b0)
@paulidale
Copy link
Contributor Author

Merged to 3.0 and master. Thanks for the review.

@paulidale paulidale closed this Nov 7, 2021
@paulidale paulidale deleted the data-race-default_DSO_meth branch November 7, 2021 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants