Skip to content

mem_alloc_test fixes#31158

Closed
esyr wants to merge 2 commits into
openssl:masterfrom
esyr:esyr/mem_alloc_test-fixes
Closed

mem_alloc_test fixes#31158
esyr wants to merge 2 commits into
openssl:masterfrom
esyr:esyr/mem_alloc_test-fixes

Conversation

@esyr
Copy link
Copy Markdown
Member

@esyr esyr commented May 12, 2026

This patch set addresses the following issues:

  • increased previously insufficient requested allocation size to trigger OOM malloc error path;
  • behaviour of my_malloc and my_realloc is updated to match behaviour of CRYPTO_malloc and CRYPTO_realloc more closely.

@esyr esyr 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 branch: 3.6 Applies to openssl-3.6 branch: 4.0 Applies to openssl-4.0 labels May 12, 2026
@esyr esyr force-pushed the esyr/mem_alloc_test-fixes branch 3 times, most recently from 937ba43 to 0c6adee Compare May 12, 2026 16:28
npajkovsky
npajkovsky reviewed May 12, 2026
View reviewed changes
Comment thread test/mem_alloc_test.c Outdated
SIZE_MAX - SQRT_SIZE_T + SQSQRT_SIZE_T,
SIZE_MAX - SQRT_SIZE_T + SQSQRT_SIZE_T,
EXP_OOM, EXP_INT_OF
},
Copy link
Copy Markdown

@npajkovsky npajkovsky May 12, 2026

Choose a reason for hiding this comment

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

I would not try to figure out the formula that tries to overflow size_t to the point where it expects oom, and rather simply remove it.

Copy link
Copy Markdown
Member

@t8m t8m May 13, 2026

Choose a reason for hiding this comment

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

Yeah, this is IMO too fragile and not sure why do we need such test case.

esyr added 2 commits May 13, 2026 15:09
@esyr
test/mem_alloc_test.c: increase alloc size for triggering OOM malloc …
d7f3e90 
…failure

While half of the address space is enough to make malloc() fail on
kernels of the same bitness, 32-bit compat can have enough to accommodate
such a request, and even three fourth of the address space.  Bump
the requested size closer to the type's upper limit.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>
@esyr
test/mem_alloc_test.c: fix my_malloc/my_realloc behaviour on size == 0
d1af253 
That puts them more in line with CRYPTO_malloc() and CRYPTO_realloc()
behaviour, whose behaviour for the requested size of 0 is well-documented
and not "implementation-defined", as POSIX allows.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>
@esyr esyr force-pushed the esyr/mem_alloc_test-fixes branch from 0c6adee to d1af253 Compare May 13, 2026 13:11
@t8m t8m added the tests: present The PR has suitable tests present label May 13, 2026
@openssl-machine openssl-machine added approval: done This pull request has the required number of approvals approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: review pending This pull request needs review by a committer approval: done This pull request has the required number of approvals labels May 13, 2026
@openssl-machine
Copy link
Copy Markdown
Collaborator

openssl-machine commented May 14, 2026

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request May 18, 2026
@esyr @jogme
test/mem_alloc_test.c: increase alloc size for triggering OOM malloc …
ff774d2 
…failure

While half of the address space is enough to make malloc() fail on
kernels of the same bitness, 32-bit compat can have enough to accommodate
such a request, and even three fourth of the address space.  Bump
the requested size closer to the type's upper limit.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Mon May 18 07:31:04 2026
(Merged from #31158)
openssl-machine pushed a commit that referenced this pull request May 18, 2026
@esyr @jogme
test/mem_alloc_test.c: fix my_malloc/my_realloc behaviour on size == 0
a9a3763 
That puts them more in line with CRYPTO_malloc() and CRYPTO_realloc()
behaviour, whose behaviour for the requested size of 0 is well-documented
and not "implementation-defined", as POSIX allows.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Mon May 18 07:31:04 2026
(Merged from #31158)
openssl-machine pushed a commit that referenced this pull request May 18, 2026
@esyr @jogme
test/mem_alloc_test.c: increase alloc size for triggering OOM malloc …
95acbe6 
…failure

While half of the address space is enough to make malloc() fail on
kernels of the same bitness, 32-bit compat can have enough to accommodate
such a request, and even three fourth of the address space.  Bump
the requested size closer to the type's upper limit.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Mon May 18 07:30:48 2026
(Merged from #31158)
openssl-machine pushed a commit that referenced this pull request May 18, 2026
@esyr @jogme
test/mem_alloc_test.c: fix my_malloc/my_realloc behaviour on size == 0
9a6af0f 
That puts them more in line with CRYPTO_malloc() and CRYPTO_realloc()
behaviour, whose behaviour for the requested size of 0 is well-documented
and not "implementation-defined", as POSIX allows.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Mon May 18 07:30:49 2026
(Merged from #31158)
@jogme
Copy link
Copy Markdown
Contributor

jogme commented May 18, 2026

Merged to the labeled branches. Thank you!

@jogme jogme closed this May 18, 2026
openssl-machine pushed a commit that referenced this pull request May 18, 2026
@esyr @jogme
test/mem_alloc_test.c: increase alloc size for triggering OOM malloc …
c885347 
…failure

While half of the address space is enough to make malloc() fail on
kernels of the same bitness, 32-bit compat can have enough to accommodate
such a request, and even three fourth of the address space.  Bump
the requested size closer to the type's upper limit.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Mon May 18 07:31:16 2026
(Merged from #31158)
openssl-machine pushed a commit that referenced this pull request May 18, 2026
@esyr @jogme
test/mem_alloc_test.c: fix my_malloc/my_realloc behaviour on size == 0
7b1bb29 
That puts them more in line with CRYPTO_malloc() and CRYPTO_realloc()
behaviour, whose behaviour for the requested size of 0 is well-documented
and not "implementation-defined", as POSIX allows.

Fixes: d090695 "test: add a sanity test for memory allocation functions"
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Mon May 18 07:31:17 2026
(Merged from #31158)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@t8m t8m t8m approved these changes

@npajkovsky npajkovsky npajkovsky approved these changes

Assignees

No one assigned

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.6 Applies to openssl-3.6 branch: 4.0 Applies to openssl-4.0 tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@esyr @openssl-machine @jogme @t8m @npajkovsky