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.

Sign up for GitHub

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

Jump to bottom

openssl#20299: Fixed use after free bug #20300

Closed
wants to merge 1 commit into from
Closed

openssl#20299: Fixed use after free bug #20300

wants to merge 1 commit into from

Conversation

blazmonster
Copy link
Contributor

@blazmonster blazmonster commented Feb 15, 2023 •
edited

CLA: trivial

mattcaswell
mattcaswell approved these changes Feb 16, 2023
View reviewed changes
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is trivial

@mattcaswell mattcaswell added branch: master Merge to master branch approval: review pending This pull request needs review by a committer cla: trivial One of the commits is marked as 'CLA: trivial' labels Feb 16, 2023
@tmshort
Copy link
Contributor

tmshort commented Feb 16, 2023

This cleanup seems wrong. ossl_ssl_init() can fail in two places, the later of which has a lock, and that lock is not freed in the case of an error, and neither is the SSL_CTX up ref.

@tmshort
Copy link
Contributor

tmshort commented Feb 16, 2023

This should be OK, assuming we include #20316

@tmshort tmshort 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 Feb 16, 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 Feb 17, 2023
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@tmshort tmshort self-assigned this Feb 20, 2023
openssl-machine pushed a commit that referenced this pull request Feb 20, 2023
@tmshort
#20299: Fixed use after free bug
c4a44e7 
CLA: trivial

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from #20300)
@tmshort
Copy link
Contributor

tmshort commented Feb 20, 2023

Merged to master, thank you for your contribution!

@tmshort tmshort closed this Feb 20, 2023
@tmshort tmshort mentioned this pull request Feb 20, 2023
Closed
@blazmonster
Copy link
Contributor Author

@tmshort thank you so much for giving me such opportunity to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattcaswell mattcaswell mattcaswell approved these changes

@tmshort tmshort tmshort approved these changes

Assignees

@tmshort tmshort

Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch cla: trivial One of the commits is marked as 'CLA: trivial'
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@blazmonster @tmshort @openssl-machine @mattcaswell