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

Urgent CI failure fix on 3.3 branch #24337

Closed
wants to merge 2 commits into from
Closed

Conversation

t8m
Copy link
Member

@t8m t8m commented May 6, 2024

This is basically a clean cherry pick of #23462 to 3.3.

1591471
1591474
1591476

which pertain to memory leaks in the conf_mod code

If an error is encountered after the module STACK_OF is duplicated or
created in the new_modules variable, we need to remember to free it in
the error path

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#23462)
Coverity caught the following issues:
1591477
1591475
1591473
1591470

all of which are simmilar, in that they catch potential divide by zero
in double values.  It can't actually happen since the the threads which
increment these counters don't exit until they reach non-zero values,
but its easy to add the checks, so lets do that to ensure that we don't
change something in the future that causes it.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#23462)
@t8m t8m 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 branch: 3.3 Merge to openssl-3.3 severity: urgent Fixes an urgent issue (exempt from 24h grace period) tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug labels May 6, 2024
@nhorman
Copy link
Contributor

nhorman commented May 6, 2024

concur urgent

@t8m t8m removed the approval: review pending This pull request needs review by a committer label May 6, 2024
@paulidale paulidale added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: otc review pending This pull request needs review by an OTC member labels May 6, 2024
@paulidale
Copy link
Contributor

agree urgent, but I can't seem to be able to merge it.

@t8m
Copy link
Member Author

t8m commented May 7, 2024

agree urgent, but I can't seem to be able to merge it.

Why weren't you able to merge it?

@t8m
Copy link
Member Author

t8m commented May 7, 2024

Ah I think I understand - because the original author of the commits is Neil so he in the formal sense cannot provide a review. But I can provide that myself.

I've merged it to the 3.3 branch. Thank you for the reviews.

@t8m t8m closed this May 7, 2024
openssl-machine pushed a commit that referenced this pull request May 7, 2024
1591471
1591474
1591476

which pertain to memory leaks in the conf_mod code

If an error is encountered after the module STACK_OF is duplicated or
created in the new_modules variable, we need to remember to free it in
the error path

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #24337)
openssl-machine pushed a commit that referenced this pull request May 7, 2024
Coverity caught the following issues:
1591477
1591475
1591473
1591470

all of which are simmilar, in that they catch potential divide by zero
in double values.  It can't actually happen since the the threads which
increment these counters don't exit until they reach non-zero values,
but its easy to add the checks, so lets do that to ensure that we don't
change something in the future that causes it.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #24337)
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: 3.3 Merge to openssl-3.3 severity: urgent Fixes an urgent issue (exempt from 24h grace period) 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

4 participants