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

OpenSSL dlloads node static linked executable #28045

Closed
wants to merge 1 commit into from

Conversation

@lal12
Copy link
Contributor

commented Jun 4, 2019

Fix: OpenSSL dlloads itself to prevent unloading, in case it might be dynamically loaded via dload. However when linked statically this will lead to dloading the main executable, which is not valid.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
    (issues with test, see: #27856)
  • commit message follows commit guidelines

See more here: #21848

common.gypi Outdated Show resolved Hide resolved
@BridgeAR

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

@lal12 do you know if this might also fix #21845?

@lal12

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

@lal12 do you know if this might also fix #21845?

Hmm in my case node wasn't compiled as PIE, therefore the dlopen failed all together, and it didn't came that far. However without looking deeply into it, I guess the whole problematic code path should not be used, therefore it is probably ok to close the ticket.

deps: OpenSSL: OpenSSL dlloads node static linked executable
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: #27925
Refs: #21848 (comment)

@lal12 lal12 force-pushed the lal12:master branch from b7d0819 to 519a3e9 Jun 4, 2019

@lal12

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

Btw this fix applies to OpenSSL >= v1.1.1b.

@nodejs-github-bot

This comment has been minimized.

@Trott
Trott approved these changes Jul 30, 2019
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@Trott

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

Landed in 1567461

@Trott Trott closed this Jul 30, 2019

Trott added a commit to Trott/io.js that referenced this pull request Jul 30, 2019
deps: dlloads node static linked executable
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 added a commit that referenced this pull request Aug 2, 2019
deps: dlloads node static linked executable
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>
@BridgeAR BridgeAR referenced this pull request Aug 6, 2019
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
deps: dlloads node static linked executable
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>
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
deps: dlloads node static linked executable
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.