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

deps: backport crypto/init.c fix for openssl110 #21848

Closed
wants to merge 1 commit into from

Conversation

LionNatsu
Copy link

The statically linked program should not try to keep loaded
by dlopen trick.

Fixes: #21845
Refs: openssl/openssl#6725

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

The statically linked program should not try to keep loaded
by dlopen trick.

Fixes: nodejs#21845
Refs: openssl/openssl#6725
@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label Jul 17, 2018
@ryzokuken
Copy link
Contributor

/cc @nodejs/crypto @tniessen

@ryzokuken ryzokuken requested a review from tniessen July 17, 2018 17:53
@addaleax addaleax added the blocked PRs that are blocked by other issues or PRs. label Jul 23, 2018
@addaleax
Copy link
Member

I’m going to label this as “blocked” until the upstream PR is merged – thanks for taking care of this issue!

@ryzokuken
Copy link
Contributor

Closing this since the upstream PR seems to be stalled. Please feel free to reopen this.

@ryzokuken ryzokuken closed this Oct 27, 2018
@lal12
Copy link
Contributor

lal12 commented Jun 3, 2019

There seems to have been a fix for that (see OPENSSL_NO_PINSHARED) in the newest openssl version. However this define does not seem to be used in the nodejs gyp build system.

@sam-github
Copy link
Contributor

@lal12 do you care to make a PR? Just adding a define to the gyp file should be straightforward.

I can even do it, but I'd need way more info.

I gather you suggest adding OPENSSL_NO_PINSHARED? But only for Node.js 8.x, since 10.x and higher use openssl 1.1.1? Though 10.x supports being built against an external openssl 1.1.0, so it may apply.

/cc @shigeki @nodejs/crypto

@lal12
Copy link
Contributor

lal12 commented Jun 3, 2019 via email

@sam-github
Copy link
Contributor

sam-github commented Jun 3, 2019 via email

lal12 added a commit to lal12/node that referenced this pull request Jun 4, 2019
OpenSSL dlloads itself to prevent unloading, in case it might be dynamically loaded. However when linked statically this will lead to dloading the main executable.

Fixes: nodejs#27925
Refs: nodejs#21848 (comment)
Trott pushed a commit to Trott/io.js that referenced this pull request Jul 30, 2019
OpenSSL dlloads itself to prevent unloading, in case it might be
dynamically loaded. However when linked statically this will lead to
dloading the main executable.

Refs: nodejs#21848 (comment)
PR-URL: nodejs#28045
Fixes: nodejs#27925
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Aug 2, 2019
OpenSSL dlloads itself to prevent unloading, in case it might be
dynamically loaded. However when linked statically this will lead to
dloading the main executable.

Refs: #21848 (comment)
PR-URL: #28045
Fixes: #27925
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
AshCripps pushed a commit to AshCripps/node that referenced this pull request Oct 17, 2019
OpenSSL dlloads itself to prevent unloading, in case it might be
dynamically loaded. However when linked statically this will lead to
dloading the main executable.

Refs: nodejs#21848 (comment)
Fixes: nodejs#29992
PR-URL: nodejs#28045
Fixes: nodejs#27925
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs pushed a commit that referenced this pull request Oct 17, 2019
OpenSSL dlloads itself to prevent unloading, in case it might be
dynamically loaded. However when linked statically this will lead to
dloading the main executable.

Refs: #21848 (comment)
Fixes: #29992

Backport-PR-URL: #30005
PR-URL: #28045
Fixes: #27925
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@LionNatsu LionNatsu deleted the patch-dlopen branch January 2, 2020 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

node crashes with "free(): invalid pointer" when exiting
6 participants