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.

Sign up for GitHub

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

Jump to bottom

deps: cherry-pick b87d408 from upstream V8 #24272

Closed
wants to merge 1 commit into from
Closed

deps: cherry-pick b87d408 from upstream V8 #24272

wants to merge 1 commit into from

Conversation

psmarshall
Copy link
Contributor

Original commit message:

[heap-profiler] Fix a use-after-free when snapshots are deleted

If a caller starts the sampling heap profiler and takes a snapshot, and
then deletes the snapshot before the sampling has completed, a
use-after-free will occur on the StringsStorage pointer.

The same issue applies for StartTrackingHeapObjects which shares the
same StringsStorage object.

Bug: v8:8373
Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
Reviewed-on: https://chromium-review.googlesource.com/c/1301477
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57114}

Refs: v8/v8@b87d408

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

@psmarshall
deps: cherry-pick b87d408 from upstream V8
3c82202 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot, and
    then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

Refs: v8/v8@b87d408
@nodejs-github-bot
Copy link
Collaborator

@psmarshall build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1555/pipeline

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v10.x v8 engine Issues and PRs related to the V8 dependency. labels Nov 9, 2018
@psmarshall
Copy link
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/18461/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1832/

@psmarshall psmarshall added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 9, 2018
@psmarshall
Copy link
Contributor Author

The version number is bumped twice because I expect to land #23827 first

@targos
Copy link
Member

targos commented Nov 9, 2018

Is this commit already on master ?

@psmarshall
Copy link
Contributor Author

It's in V8 7.2 so I guess not - should I just retarget it to master? We aren't merging it back in v8

@targos
Copy link
Member

targos commented Nov 9, 2018

should I just retarget it to master?

Yes, please. We will also have to take it in v11.x and it's easier to manage if the fix lands first on master.

@danbev
Copy link
Contributor

danbev commented Nov 12, 2018

I updated v8_embedder_string to -node.11 which seemed like the right thing to do.
Landed in a8847aa.

@danbev danbev closed this Nov 12, 2018
danbev pushed a commit that referenced this pull request Nov 12, 2018
@psmarshall @danbev
deps: cherry-pick b87d408 from upstream V8
a8847aa 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot,
    and then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

PR-URL: #24272
Refs:
v8/v8@b87d408
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@targos targos removed the v10.x label Nov 12, 2018
@psmarshall
Copy link
Contributor Author

Thanks, the patch should apply cleanly to 10. This should also be backported to 8 if possible.

@richardlau
Copy link
Member

A bit late to this, but since the PR was not retargetted doesn't this mean it effectively landed without relevant CI runs?

@danbev
Copy link
Contributor

danbev commented Nov 13, 2018

A bit late to this, but since the PR was not retargetted doesn't this mean it effectively landed without relevant CI runs?

Sorry about this, that was my fault. I read the comments a little too quickly and did not notice it was still targeted at v10.x-staging. If this is causing issues please let me know and I'll revert. Thanks

BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
@psmarshall @BridgeAR
deps: cherry-pick b87d408 from upstream V8
a39493f 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot,
    and then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

PR-URL: #24272
Refs:
v8/v8@b87d408
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Nov 14, 2018
Merged
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
@psmarshall @kiyomizumia
deps: cherry-pick b87d408 from upstream V8
b9a11d1 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot,
    and then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

PR-URL: nodejs#24272
Refs:
v8/v8@b87d408
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This was referenced Nov 15, 2018
Closed
Closed
Closed
Closed
Closed
Closed
targos pushed a commit to targos/node that referenced this pull request Dec 4, 2018
@psmarshall @targos
deps: cherry-pick b87d408 from upstream V8
bbcf8e6 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot,
    and then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

PR-URL: nodejs#24272
Refs:
v8/v8@b87d408
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
pull bot pushed a commit to shakir-abdo/node that referenced this pull request Dec 6, 2018
@psmarshall @targos
deps: cherry-pick b87d408 from upstream V8
e36e9dd 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot,
    and then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

PR-URL: nodejs#24272
Refs:
v8/v8@b87d408
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
codebytere pushed a commit that referenced this pull request Jan 12, 2019
@psmarshall @codebytere
deps: cherry-pick b87d408 from upstream V8
733cb1e 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot,
    and then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

PR-URL: #24272
Refs:
v8/v8@b87d408
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@psmarshall @refack
deps: cherry-pick b87d408 from upstream V8
8d94aa9 
Original commit message:

    [heap-profiler] Fix a use-after-free when snapshots are deleted

    If a caller starts the sampling heap profiler and takes a snapshot,
    and then deletes the snapshot before the sampling has completed, a
    use-after-free will occur on the StringsStorage pointer.

    The same issue applies for StartTrackingHeapObjects which shares the
    same StringsStorage object.

    Bug: v8:8373
    Change-Id: I5d69d60d3f9465f9dd3b3bef107c204e0fda0643
    Reviewed-on: https://chromium-review.googlesource.com/c/1301477
    Commit-Queue: Peter Marshall <petermarshall@chromium.org>
    Reviewed-by: Alexei Filippov <alph@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57114}

PR-URL: nodejs#24272
Refs:
v8/v8@b87d408
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@codebytere codebytere mentioned this pull request Jan 15, 2019
Merged
This was referenced Jan 29, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@danbev danbev danbev approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@psmarshall @nodejs-github-bot @targos @danbev @richardlau @cjihrig