Skip to content

Comments

Fix memory leak in error path of ec_gen_init()#29335

Closed
ndossche wants to merge 1 commit intoopenssl:masterfrom
ndossche:fix-ec-gen-init
Closed

Fix memory leak in error path of ec_gen_init()#29335
ndossche wants to merge 1 commit intoopenssl:masterfrom
ndossche:fix-ec-gen-init

Conversation

@ndossche
Copy link
Contributor

@ndossche ndossche commented Dec 8, 2025

ec_gen_set_params() can fail after some big numbers have already been copied over. Those need to be cleaned to avoid a memory leak on failure. This can be done with ec_gen_cleanup(), which is also consistent in how the ecx_gen code does it.

Note: This was detected using an experimental static analyzer our group is working on. Although I did manual verification it's always possible this is a false alarm or not important.

ec_gen_set_params() can fail after some big numbers have already been
copied over. Those need to be cleaned to avoid a memory leak on failure.
This can be done with ec_gen_cleanup(), which is also consistent in how
the ecx_gen code does it.
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Dec 8, 2025
@t8m t8m added branch: master Applies to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing branch: 3.3 Applies to openssl-3.3 branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 labels Dec 8, 2025
Copy link
Member

@beldmit beldmit left a comment

Choose a reason for hiding this comment

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

LGTM

@beldmit beldmit 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 Dec 8, 2025
@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 Dec 9, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m t8m closed this Dec 11, 2025
@t8m t8m reopened this Dec 11, 2025
openssl-machine pushed a commit that referenced this pull request Dec 11, 2025
ec_gen_set_params() can fail after some big numbers have already been
copied over. Those need to be cleaned to avoid a memory leak on failure.
This can be done with ec_gen_cleanup(), which is also consistent in how
the ecx_gen code does it.

Reviewed-by: Dmitry Belyavskiy <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29335)

(cherry picked from commit 26d138a)
openssl-machine pushed a commit that referenced this pull request Dec 11, 2025
ec_gen_set_params() can fail after some big numbers have already been
copied over. Those need to be cleaned to avoid a memory leak on failure.
This can be done with ec_gen_cleanup(), which is also consistent in how
the ecx_gen code does it.

Reviewed-by: Dmitry Belyavskiy <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29335)

(cherry picked from commit 26d138a)
openssl-machine pushed a commit that referenced this pull request Dec 11, 2025
ec_gen_set_params() can fail after some big numbers have already been
copied over. Those need to be cleaned to avoid a memory leak on failure.
This can be done with ec_gen_cleanup(), which is also consistent in how
the ecx_gen code does it.

Reviewed-by: Dmitry Belyavskiy <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29335)

(cherry picked from commit 26d138a)
openssl-machine pushed a commit that referenced this pull request Dec 11, 2025
ec_gen_set_params() can fail after some big numbers have already been
copied over. Those need to be cleaned to avoid a memory leak on failure.
This can be done with ec_gen_cleanup(), which is also consistent in how
the ecx_gen code does it.

Reviewed-by: Dmitry Belyavskiy <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29335)
@t8m
Copy link
Member

t8m commented Dec 11, 2025

Merged to the master, 3.6, 3.5, 3.4 and 3.3 branches. Thank you for your contribution.

@t8m t8m closed this Dec 11, 2025
openssl-machine pushed a commit that referenced this pull request Dec 11, 2025
ec_gen_set_params() can fail after some big numbers have already been
copied over. Those need to be cleaned to avoid a memory leak on failure.
This can be done with ec_gen_cleanup(), which is also consistent in how
the ecx_gen code does it.

Reviewed-by: Dmitry Belyavskiy <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #29335)

(cherry picked from commit 26d138a)
@github-actions github-actions bot removed the severity: fips change The pull request changes FIPS provider sources label Dec 11, 2025
cxx194832 pushed a commit to cxx194832/openssl that referenced this pull request Dec 12, 2025
ec_gen_set_params() can fail after some big numbers have already been
copied over. Those need to be cleaned to avoid a memory leak on failure.
This can be done with ec_gen_cleanup(), which is also consistent in how
the ecx_gen code does it.

Reviewed-by: Dmitry Belyavskiy <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#29335)
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 branch: 3.3 Applies to openssl-3.3 branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 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.

4 participants