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 mem function documentation and some return codes for mem debug functions #18967

Closed
wants to merge 2 commits into from

Conversation

mattcaswell
Copy link
Member

The documentation was misleading in that it suggests that the OPENSSL_MALLOC_FD environment variable will record information about all allocations. While this is true it doesn't record the most useful information that you might expect such as the requested size of the allocation! It is mainly for use in conjunction with OPENSSL_MALLOC_FAILURES, and reports information about what chance an allocation has of failing.

Additionally we fix the return codes for the CRYPTO_mem_debug_push() and CRYPTO_mem_debug_pop() functions. Those 2 functions historically only ever returned 0 or 1. In OpenSSL 3.0 they were made no-ops and the documentation says they always return 0. In fact they were returning -1. If any application was actually using these functions then it may appear that they were actually successful (e.g. -1 could be interpreted as "true").

The documentation was misleading in that it suggests that this environment
variable will record information about all allocations. While this is true
it doesn't record the most useful information that you might expect such
as the requested size of the allocation! It is mainly for use in
conjunction with OPENSSL_MALLOC_FAILURES, and reports information about
what chance an allocation has of failing.

We also clarify that the mem_debug functions are actually no-ops in 3.0.
Those 2 functions historically only ever returned 0 or 1. In OpenSSL 3.0
they were made no-ops and the documentation says they always return 0. In
fact they were returning -1. If any application was actually using these
functions then it may appear that they were actually successful (e.g. -1
could be interpreted as "true").
@mattcaswell mattcaswell added branch: master Merge to master branch 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.0 Merge to openssl-3.0 branch labels Aug 9, 2022
@hlandau hlandau removed the approval: review pending This pull request needs review by a committer label Aug 11, 2022
@t8m t8m added triaged: bug The issue/pr is/fixes a bug triaged: documentation The issue/pr deals with documentation (errors) labels Aug 16, 2022
@t8m t8m 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 Aug 16, 2022
@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 17, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m
Copy link
Member

t8m commented Aug 17, 2022

Merged to master and 3.0 branches. Thank you.

@t8m t8m closed this Aug 17, 2022
openssl-machine pushed a commit that referenced this pull request Aug 17, 2022
The documentation was misleading in that it suggests that this environment
variable will record information about all allocations. While this is true
it doesn't record the most useful information that you might expect such
as the requested size of the allocation! It is mainly for use in
conjunction with OPENSSL_MALLOC_FAILURES, and reports information about
what chance an allocation has of failing.

We also clarify that the mem_debug functions are actually no-ops in 3.0.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18967)
openssl-machine pushed a commit that referenced this pull request Aug 17, 2022
Those 2 functions historically only ever returned 0 or 1. In OpenSSL 3.0
they were made no-ops and the documentation says they always return 0. In
fact they were returning -1. If any application was actually using these
functions then it may appear that they were actually successful (e.g. -1
could be interpreted as "true").

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18967)
openssl-machine pushed a commit that referenced this pull request Aug 17, 2022
The documentation was misleading in that it suggests that this environment
variable will record information about all allocations. While this is true
it doesn't record the most useful information that you might expect such
as the requested size of the allocation! It is mainly for use in
conjunction with OPENSSL_MALLOC_FAILURES, and reports information about
what chance an allocation has of failing.

We also clarify that the mem_debug functions are actually no-ops in 3.0.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18967)

(cherry picked from commit 2c35d61)
openssl-machine pushed a commit that referenced this pull request Aug 17, 2022
Those 2 functions historically only ever returned 0 or 1. In OpenSSL 3.0
they were made no-ops and the documentation says they always return 0. In
fact they were returning -1. If any application was actually using these
functions then it may appear that they were actually successful (e.g. -1
could be interpreted as "true").

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18967)

(cherry picked from commit f868454)
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
The documentation was misleading in that it suggests that this environment
variable will record information about all allocations. While this is true
it doesn't record the most useful information that you might expect such
as the requested size of the allocation! It is mainly for use in
conjunction with OPENSSL_MALLOC_FAILURES, and reports information about
what chance an allocation has of failing.

We also clarify that the mem_debug functions are actually no-ops in 3.0.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#18967)
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
Those 2 functions historically only ever returned 0 or 1. In OpenSSL 3.0
they were made no-ops and the documentation says they always return 0. In
fact they were returning -1. If any application was actually using these
functions then it may appear that they were actually successful (e.g. -1
could be interpreted as "true").

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#18967)
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 triaged: bug The issue/pr is/fixes a bug triaged: documentation The issue/pr deals with documentation (errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants