Skip to content

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

Closed
ihciah wants to merge 1 commit intoopenssl:masterfrom
ihciah:bio-fix
Closed

bio fix: pass flags on BIO_ctrl to make flush retriable#21298
ihciah wants to merge 1 commit intoopenssl:masterfrom
ihciah:bio-fix

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 Applies to master branch approval: review pending This pull request needs review by a committer approval: otc review pending triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing labels 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 Applies 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.

6 participants