Skip to content

Comments

Fix the "Out of memory" EVP KDF scrypt test#16444

Closed
bernd-edlinger wants to merge 2 commits intoopenssl:masterfrom
bernd-edlinger:fix_kdf_scrypt_test
Closed

Fix the "Out of memory" EVP KDF scrypt test#16444
bernd-edlinger wants to merge 2 commits intoopenssl:masterfrom
bernd-edlinger:fix_kdf_scrypt_test

Conversation

@bernd-edlinger
Copy link
Member

@bernd-edlinger bernd-edlinger commented Aug 27, 2021

This test did not really execute, since usually
the OPENSSL_malloc(0) will fail and prevent the
execution of the KDF.

Checklist
  • documentation is added or updated
  • tests are added or updated

This test did not really execute, since usually
the OPENSSL_malloc(0) will fail and prevent the
execution of the KDF.
@bernd-edlinger bernd-edlinger added the branch: master Applies to master branch label Aug 27, 2021
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

Hmmm, I thought malloc(0) was well defined. Still this is better.

@paulidale paulidale added the approval: done This pull request has the required number of approvals label Aug 27, 2021
@bernd-edlinger
Copy link
Member Author

I just thought, what if someone uses CRYPTO_set_mem_functions to override the default.
For the record: malloc(0) is implementation dependent. it may also return a "unique" memory objec of size 0.

@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Aug 27, 2021
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

openssl-machine pushed a commit that referenced this pull request Aug 28, 2021
This test did not really execute, since usually
the OPENSSL_malloc(0) will fail and prevent the
execution of the KDF.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from #16444)
@bernd-edlinger
Copy link
Member Author

Merged to master as cce935b. Thanks!

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

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants