X Tutup
The Wayback Machine - https://web.archive.org/web/20220323011220/https://github.com/nodejs/node/pull/35492
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

src: limit GetProcessTitle() result to 1MB #35492

Closed
wants to merge 2 commits into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Oct 4, 2020

GetProcessTitle() otherwise runs an infinite loop when
uv_setup_argv() has not been called (yet). This is a problem
e.g. in assertions from static constructors, which run before
main() and thus before argc and argv become available.

To solve that, do not allocate more than 1MB of storage for the
title and bail out if we reach that point.

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

`GetProcessTitle()` otherwise runs an infinite loop when
`uv_setup_argv()` has not been called (yet). This is a problem
e.g. in assertions from static constructors, which run before
`main()` and thus before `argc` and `argv` become available.

To solve that, do not allocate more than 1MB of storage for the
title and bail out if we reach that point.
@nodejs-github-bot nodejs-github-bot added the c++ label Oct 4, 2020
jasnell
jasnell approved these changes Oct 4, 2020
devsnek
devsnek approved these changes Oct 4, 2020
@addaleax addaleax added author ready request-ci labels Oct 4, 2020
@github-actions github-actions bot removed the request-ci label Oct 4, 2020
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Oct 4, 2020

Trott
Trott approved these changes Oct 4, 2020
cjihrig
cjihrig approved these changes Oct 4, 2020
Copy link
Contributor

@cjihrig cjihrig left a comment

LGTM. An accompanying comment might be helpful though.

@addaleax
Copy link
Member Author

@addaleax addaleax commented Oct 4, 2020

@cjihrig Added a comment 👍

@addaleax addaleax added the request-ci label Oct 4, 2020
@github-actions github-actions bot removed the request-ci label Oct 4, 2020
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Oct 4, 2020

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Oct 5, 2020

addaleax added a commit that referenced this issue Oct 6, 2020
`GetProcessTitle()` otherwise runs an infinite loop when
`uv_setup_argv()` has not been called (yet). This is a problem
e.g. in assertions from static constructors, which run before
`main()` and thus before `argc` and `argv` become available.

To solve that, do not allocate more than 1MB of storage for the
title and bail out if we reach that point.

PR-URL: #35492
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax
Copy link
Member Author

@addaleax addaleax commented Oct 6, 2020

Landed in 18af0b0

@addaleax addaleax closed this Oct 6, 2020
@addaleax addaleax deleted the process-title-bailout branch Oct 6, 2020
@danielleadams danielleadams mentioned this pull request Oct 6, 2020
danielleadams added a commit that referenced this issue Oct 6, 2020
`GetProcessTitle()` otherwise runs an infinite loop when
`uv_setup_argv()` has not been called (yet). This is a problem
e.g. in assertions from static constructors, which run before
`main()` and thus before `argc` and `argv` become available.

To solve that, do not allocate more than 1MB of storage for the
title and bail out if we reach that point.

PR-URL: #35492
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
joesepi added a commit to joesepi/node that referenced this issue Jan 8, 2021
`GetProcessTitle()` otherwise runs an infinite loop when
`uv_setup_argv()` has not been called (yet). This is a problem
e.g. in assertions from static constructors, which run before
`main()` and thus before `argc` and `argv` become available.

To solve that, do not allocate more than 1MB of storage for the
title and bail out if we reach that point.

PR-URL: nodejs#35492
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready c++
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
X Tutup