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

bio fix: pass flags on BIO_ctrl to make flush retriable #21298

Closed
wants to merge 1 commit into from

Conversation

ihciah
Copy link
Contributor

@ihciah ihciah commented Jun 27, 2023

This PR fixed a wrong wbio pointer usage on SSL_get_error.
The bug will cause SSL_get_error to return SSL_ERROR_SYSCALL even after BIO_set_retry_write in some cases such as BIO_CTRL_FLUSH. Without this fix, when BIO_CTRL_FLUSH, if BIO writer gets a WOULD_BLOCK error, it calls BIO_set_retry_write and returns 0, it will not work and openssl will return a failure.

Previous review is in #20919 , I reopened this PR to trigger CLA checking.

Co-authored-by: suikammd <suikalala@gmail.com>
@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing labels Jun 27, 2023
@paulidale paulidale removed the approval: otc review pending This pull request needs review by an OTC member label Jun 27, 2023
@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

@ihciah
Copy link
Contributor Author

ihciah commented Aug 2, 2023

Hi everyone, is everything alright?

@mattcaswell mattcaswell 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 Aug 2, 2023
@mattcaswell mattcaswell closed this Aug 2, 2023
@mattcaswell mattcaswell reopened this Aug 2, 2023
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Aug 3, 2023
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@tmshort tmshort self-assigned this Aug 4, 2023
@tmshort
Copy link
Contributor

tmshort commented Aug 4, 2023

Merged to master. Thank you for your contribution!

@tmshort tmshort closed this Aug 4, 2023
openssl-machine pushed a commit that referenced this pull request Aug 4, 2023
Co-authored-by: suikammd <suikalala@gmail.com>

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from #21298)
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 tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants