Skip to content

Conversation

esyr
Copy link
Member

@esyr esyr commented Aug 11, 2025

The __attribute__((malloc)) is for functions that return new memory, and "the memory [returned by the function] has undefined content", which is a property that doesn't hold for the *dup functions (the same reason it doesn't apply to realloc).

Fixes: e103595 "OSSL_CRYPTO_ALLOC attribute introduction proposal."

@esyr esyr added the triaged: bug The issue/pr is/fixes a bug label Aug 11, 2025
The __attribute__((malloc)) is for functions that return new memory,
and "the memory [returned by the function] has undefined content", which
is a property that doesn't hold for the *dup functions (the same reason
it doesn't apply to realloc).

Fixes: e103595 "OSSL_CRYPTO_ALLOC attribute introduction proposal."
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>
@esyr esyr force-pushed the esyr/memdup-no-malloc-attribute branch from 0f7c890 to 230d53f Compare August 11, 2025 09:06
@esyr esyr changed the title Remove OSSL_CRYPTO_ALLOC from CRYPTO_*dup routines Remove OSSL_CRYPTO_ALLOC attribute from CRYPTO_*dup routines Aug 11, 2025
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Aug 11, 2025
@paulidale
Copy link
Contributor

Would the alloc_size attribute work for these?

@esyr
Copy link
Member Author

esyr commented Aug 11, 2025

Yes, I'd like to introduce alloc_size, alloc_align, and allocator attributes, as well as the deallocator argument to the malloc attribute, but I gather it should be a separate PR.

@vavroch2010 vavroch2010 moved this to Waiting Review in Development Board Aug 11, 2025
@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 branch: 3.4 Merge to openssl-3.4 branch: 3.5 Merge to openssl-3.5 labels Aug 11, 2025
@t8m t8m added the tests: exempted The PR is exempt from requirements for testing label Aug 11, 2025
@github-project-automation github-project-automation bot moved this from Waiting Review to Waiting Merge in Development Board Aug 11, 2025
@nhorman nhorman 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 Aug 11, 2025
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.

A separate PR is fine.

@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 12, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Aug 12, 2025
The __attribute__((malloc)) is for functions that return new memory,
and "the memory [returned by the function] has undefined content", which
is a property that doesn't hold for the *dup functions (the same reason
it doesn't apply to realloc).

Fixes: e103595 "OSSL_CRYPTO_ALLOC attribute introduction proposal."
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28220)
openssl-machine pushed a commit that referenced this pull request Aug 12, 2025
The __attribute__((malloc)) is for functions that return new memory,
and "the memory [returned by the function] has undefined content", which
is a property that doesn't hold for the *dup functions (the same reason
it doesn't apply to realloc).

Fixes: e103595 "OSSL_CRYPTO_ALLOC attribute introduction proposal."
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28220)

(cherry picked from commit 85bba74)
openssl-machine pushed a commit that referenced this pull request Aug 12, 2025
The __attribute__((malloc)) is for functions that return new memory,
and "the memory [returned by the function] has undefined content", which
is a property that doesn't hold for the *dup functions (the same reason
it doesn't apply to realloc).

Fixes: e103595 "OSSL_CRYPTO_ALLOC attribute introduction proposal."
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28220)

(cherry picked from commit 85bba74)
openssl-machine pushed a commit that referenced this pull request Aug 12, 2025
The __attribute__((malloc)) is for functions that return new memory,
and "the memory [returned by the function] has undefined content", which
is a property that doesn't hold for the *dup functions (the same reason
it doesn't apply to realloc).

Fixes: e103595 "OSSL_CRYPTO_ALLOC attribute introduction proposal."
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28220)

(cherry picked from commit 85bba74)
@nhorman
Copy link
Contributor

nhorman commented Aug 12, 2025

merged to master, 3.5, 3.4, 3.3 and 3.2

@nhorman nhorman closed this Aug 12, 2025
@github-project-automation github-project-automation bot moved this from Waiting Merge to Done in Development Board Aug 12, 2025
openssl-machine pushed a commit that referenced this pull request Aug 12, 2025
The __attribute__((malloc)) is for functions that return new memory,
and "the memory [returned by the function] has undefined content", which
is a property that doesn't hold for the *dup functions (the same reason
it doesn't apply to realloc).

Fixes: e103595 "OSSL_CRYPTO_ALLOC attribute introduction proposal."
Signed-off-by: Eugene Syromiatnikov <esyr@openssl.org>

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #28220)

(cherry picked from commit 85bba74)
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.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 branch: 3.4 Merge to openssl-3.4 branch: 3.5 Merge to openssl-3.5 severity: fips change The pull request changes FIPS provider sources tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants