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 a possible memleak in bind_afalg #23409

Conversation

bernd-edlinger
Copy link
Member

bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy method to be called. But that does not happen
because the dynamic engine object is not destroyed in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */

bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy
method to be called.  But that does not happen
because the dynamic engine object is not destroyed
in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no
further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */
@bernd-edlinger bernd-edlinger added branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 labels Jan 28, 2024
@tom-cosgrove-arm tom-cosgrove-arm added approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member labels Jan 29, 2024
@tom-cosgrove-arm tom-cosgrove-arm added tests: exempted The PR is exempt from requirements for testing and removed approval: review pending This pull request needs review by a committer labels Jan 29, 2024
@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed approval: otc review pending This pull request needs review by an OTC member labels Jan 29, 2024
@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Jan 29, 2024
@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 Jan 30, 2024
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@mattcaswell
Copy link
Member

Pushed to master/3.2/3.1/3.0

openssl-machine pushed a commit that referenced this pull request Jan 31, 2024
bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy
method to be called.  But that does not happen
because the dynamic engine object is not destroyed
in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no
further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #23409)
openssl-machine pushed a commit that referenced this pull request Jan 31, 2024
bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy
method to be called.  But that does not happen
because the dynamic engine object is not destroyed
in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no
further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #23409)

(cherry picked from commit 729a149)
openssl-machine pushed a commit that referenced this pull request Jan 31, 2024
bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy
method to be called.  But that does not happen
because the dynamic engine object is not destroyed
in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no
further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #23409)

(cherry picked from commit 729a149)
openssl-machine pushed a commit that referenced this pull request Jan 31, 2024
bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy
method to be called.  But that does not happen
because the dynamic engine object is not destroyed
in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no
further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #23409)

(cherry picked from commit 729a149)
bernd-edlinger added a commit to bernd-edlinger/openssl that referenced this pull request Feb 1, 2024
bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy
method to be called.  But that does not happen
because the dynamic engine object is not destroyed
in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no
further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#23409)

(cherry picked from commit 729a149)
Sashan pushed a commit to Sashan/openssl that referenced this pull request Feb 12, 2024
bind_afalg calls afalg_aes_cbc which allocates
cipher_handle->_hidden global object(s)
but if one of them fails due to out of memory,
the function bind_afalg relies on the engine destroy
method to be called.  But that does not happen
because the dynamic engine object is not destroyed
in the usual way in dynamic_load in this case:

If the bind_engine function fails, there will be no
further calls into the shared object.
See ./crypto/engine/eng_dyn.c near the comment:
/* Copy the original ENGINE structure back */

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#23409)
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 branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 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

5 participants