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

Wrap in/out length pointer APIs: avoid heap escape #130

Closed
wants to merge 1 commit into from

Conversation

dagood
Copy link
Collaborator

@dagood dagood commented Nov 7, 2023

Add wrappers that help avoid escape to the heap for this list:

openssl/cgo_go122.go

Lines 10 to 17 in 61234a9

#cgo noescape go_openssl_EVP_PKEY_derive
#cgo nocallback go_openssl_EVP_PKEY_derive
#cgo noescape go_openssl_EVP_PKEY_get_raw_public_key
#cgo nocallback go_openssl_EVP_PKEY_get_raw_public_key
#cgo noescape go_openssl_EVP_PKEY_get_raw_private_key
#cgo nocallback go_openssl_EVP_PKEY_get_raw_private_key
#cgo noescape go_openssl_EVP_DigestSign
#cgo nocallback go_openssl_EVP_DigestSign

There's only a benchmark for ECDH (EVP_PKEY_derive) but this PR shows the same results as #113:

goos: linux
goarch: amd64
pkg: github.com/golang-fips/openssl/v2
cpu: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
       │   old.txt   │           new.txt            │
       │   sec/op    │   sec/op     vs base         │
ECDH-8   104.1µ ± 3%   101.2µ ± 3%  ~ (p=0.065 n=8)

       │  old.txt   │              new.txt              │
       │    B/op    │    B/op     vs base               │
ECDH-8   40.00 ± 0%   32.00 ± 0%  -20.00% (p=0.000 n=8)

       │  old.txt   │              new.txt              │
       │ allocs/op  │ allocs/op   vs base               │
ECDH-8   2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=8)

The wrapper makes it so you can't distinguish a "valid" returned 0 vs. a 0 returned because there's an error, and I don't know for sure that won't cause problems in some use cases. (E.g. make an error show up in the wrong way.)

Returning a C struct with two values is what I have in mind make a more transparent wrapper that avoids losing this difference, but I figured I'd submit this PR first in case it's not worth the complexity. Please let me know if that seems better. (The benchmark result is the same when I tried this out for EVP_PKEY_derive.)

goopenssl.h Outdated Show resolved Hide resolved
Base automatically changed from dev/dagood/rm-cgo-directives to v2 November 8, 2023 15:08
@dagood
Copy link
Collaborator Author

dagood commented Nov 8, 2023

@dagood dagood closed this Nov 8, 2023
@dagood dagood deleted the dev/dagood/heap-avoidance-wrappers branch November 8, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants