[v8.x] deps: V8: cherry-pick 64-bit hash seed commits by hashseed · Pull Request #23274 · nodejs/node

hashseed · 2018-10-05T10:38:19Z

This serves as mitigation for the so-called HashWick vulnerability.

Original commit messages:

  commit d5686a74d56fbb6985b22663ddadd66eb7b91519
    Author: Yang Guo <yangguo@chromium.org>
    Date: Mon Jul 16 11:19:42 2018

    Extend hash seed to 64 bits

    R=bmeurer@chromium.org, ulan@chromium.org

    Bug: chromium:680662
    Change-Id: I5e1486ad2a42db2998d5485a0c4e711378678e6c
    Reviewed-on: https://chromium-review.googlesource.com/1136034
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#54460}

  commit 3833fef57368c53c6170559ffa524c8c69f16ee5
    Author: Yang Guo <yangguo@chromium.org>
    Date: Thu Sep 20 11:43:13 2018

    Refactor integer hashing function names

    We now clearly differentiate between:
    - unseeded hash for 32-bit integers
    - unseeded hash for 64-bit integers
    - seeded hash for 32-bit integers
    - seeded hash for strings

    R=bmeurer@chromium.org

    Bug: chromium:680662
    Change-Id: I7459958c4158ee3501c962943dff8f33258bb5ce
    Reviewed-on: https://chromium-review.googlesource.com/1235973
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56068}

  commit 95a979e02d7154e45b293261a6998c99d71fc238
    Author: Yang Guo <yangguo@chromium.org>
    Date: Thu Sep 20 14:34:48 2018

    Call into C++ to compute seeded integer hash

    R=bmeurer@chromium.org

    Bug: chromium:680662
    Change-Id: I8dace89d576dfcc5833fd539ce698a9ade1cb5a0
    Reviewed-on: https://chromium-review.googlesource.com/1235928
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56091}

  commit 2c2af0022d5feb9e525a00a76cb15db9f3e38dba
    Author: Yang Guo <yangguo@chromium.org>
    Date: Thu Sep 27 16:37:57 2018

    Use 64-bit for seeded integer hashes

    R=petermarshall@chromium.org

    Bug: chromium:680662
    Change-Id: If48d1043dbe1e1bb695ec890c23e103a6cacf2d4
    Reviewed-on: https://chromium-review.googlesource.com/1244220
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Peter Marshall <petermarshall@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56271}

Refs: #23259

Checklist

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

nodejs-github-bot · 2018-10-05T10:38:20Z

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

mcollina

LGTM

BethGriggs · 2018-10-16T15:34:39Z

@hashseed, could you take a look at the conflicts and run CI/V8 CI please? Thanks

hashseed · 2018-10-22T10:00:51Z

I pushed a rebased version to my branch, but for some reason github is not picking up the change.

refack · 2018-10-22T14:10:47Z

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

refack · 2018-10-23T14:39:20Z

Compilation fail in Debug - https://ci.nodejs.org/job/node-test-commit-linux-containered/8019/nodes=ubuntu1604_sharedlibs_debug_x64/

10:27:16  ccache g++ '-DV8_GYP_BUILD' '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' '-DV8_ENABLE_CHECKS' '-DOBJECT_PRINT' '-DVERIFY_HEAP' '-DDEBUG' '-DV8_TRACE_MAPS' '-DENABLE_HANDLE_ZAPPING' '-DENABLE_SLOW_DCHECKS' '-D_DEBUG' -I../deps/v8 -I../. -I/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_debug_x64/out/Debug/obj/gen -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -pthread -Wall -Wextra -Wno-unused-parameter -m64 -fno-strict-aliasing -m64 -Woverloaded-virtual -fdata-sections -ffunction-sections -g -O0 -fno-rtti -fno-exceptions -std=gnu++0x -MMD -MF /home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_debug_x64/out/Debug/.deps//home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_debug_x64/out/Debug/obj.target/v8_base/deps/v8/src/profiler/profile-generator.o.d.raw   -c -o /home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_debug_x64/out/Debug/obj.target/v8_base/deps/v8/src/profiler/profile-generator.o ../deps/v8/src/profiler/profile-generator.cc
10:27:20 ../deps/v8/src/profiler/profile-generator.cc: In member function 'void v8::internal::CodeMap::AddCode(v8::internal::Address, v8::internal::CodeEntry*, unsigned int)':
10:27:20 ../deps/v8/src/profiler/profile-generator.cc:536:65: error: 'kNullAddress' was not declared in this scope

Maybe related test fails:
https://ci.nodejs.org/job/node-test-commit-linux/22565/nodes=debian8-x86/console

10:33:55 not ok 578 parallel/test-fs-buffer
10:33:55   ---
10:33:55   duration_ms: 1.416
10:33:55   severity: crashed
10:33:55   exitcode: -6
10:33:55   stack: |-
10:33:55   ...

10:35:34 not ok 1237 parallel/test-net-socket-byteswritten
10:35:34   ---
10:35:34   duration_ms: 0.408
10:35:34   severity: crashed
10:35:34   exitcode: -6
10:35:34   stack: |-
10:35:34   ...

refack · 2018-10-23T14:45:36Z

The compilation fail might have always been there... We only recently started failing CI on debug build fail.
So running baseline:
https://ci.nodejs.org/job/node-test-commit-linux-containered/8053/ (master is failing)

hashseed · 2018-10-23T18:57:45Z

That was likely a bad merge from @psmarshall when he merged all the CPU profiler goodness. I'll fix it as a driveby.

This serves as mitigation for the so-called HashWick vulnerability. Original commit messages: commit d5686a74d56fbb6985b22663ddadd66eb7b91519 Author: Yang Guo <yangguo@chromium.org> Date: Mon Jul 16 11:19:42 2018 Extend hash seed to 64 bits R=bmeurer@chromium.org, ulan@chromium.org Bug: chromium:680662 Change-Id: I5e1486ad2a42db2998d5485a0c4e711378678e6c Reviewed-on: https://chromium-review.googlesource.com/1136034 Reviewed-by: Marja Hölttä <marja@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#54460} commit 3833fef57368c53c6170559ffa524c8c69f16ee5 Author: Yang Guo <yangguo@chromium.org> Date: Thu Sep 20 11:43:13 2018 Refactor integer hashing function names We now clearly differentiate between: - unseeded hash for 32-bit integers - unseeded hash for 64-bit integers - seeded hash for 32-bit integers - seeded hash for strings R=bmeurer@chromium.org Bug: chromium:680662 Change-Id: I7459958c4158ee3501c962943dff8f33258bb5ce Reviewed-on: https://chromium-review.googlesource.com/1235973 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#56068} commit 95a979e02d7154e45b293261a6998c99d71fc238 Author: Yang Guo <yangguo@chromium.org> Date: Thu Sep 20 14:34:48 2018 Call into C++ to compute seeded integer hash R=bmeurer@chromium.org Bug: chromium:680662 Change-Id: I8dace89d576dfcc5833fd539ce698a9ade1cb5a0 Reviewed-on: https://chromium-review.googlesource.com/1235928 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#56091} commit 2c2af0022d5feb9e525a00a76cb15db9f3e38dba Author: Yang Guo <yangguo@chromium.org> Date: Thu Sep 27 16:37:57 2018 Use 64-bit for seeded integer hashes R=petermarshall@chromium.org Bug: chromium:680662 Change-Id: If48d1043dbe1e1bb695ec890c23e103a6cacf2d4 Reviewed-on: https://chromium-review.googlesource.com/1244220 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#56271} Refs: nodejs#23259

hashseed · 2018-10-23T18:59:18Z

I made a drive-by fix which replaces kNullAddress with 0.

refack · 2018-10-23T19:14:45Z

Thanks!
https://ci.nodejs.org/job/node-test-commit-linux-containered/8057/nodes=ubuntu1604_sharedlibs_debug_x64/

hashseed · 2018-10-25T06:00:17Z

Is this good for merging yet?

refack · 2018-10-25T13:23:01Z

Is this good for merging yet?

I think it's customary for a releaser to merge into version branches.
/CC @nodejs/release

BethGriggs · 2018-10-30T15:27:42Z

Linux CI: https://ci.nodejs.org/job/node-test-commit-linux/22758/ [Passed]

This serves as mitigation for the so-called HashWick vulnerability. Original commit messages: commit d5686a74d56fbb6985b22663ddadd66eb7b91519 Author: Yang Guo <yangguo@chromium.org> Date: Mon Jul 16 11:19:42 2018 Extend hash seed to 64 bits R=bmeurer@chromium.org, ulan@chromium.org Bug: chromium:680662 Change-Id: I5e1486ad2a42db2998d5485a0c4e711378678e6c Reviewed-on: https://chromium-review.googlesource.com/1136034 Reviewed-by: Marja Hölttä <marja@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#54460} commit 3833fef57368c53c6170559ffa524c8c69f16ee5 Author: Yang Guo <yangguo@chromium.org> Date: Thu Sep 20 11:43:13 2018 Refactor integer hashing function names We now clearly differentiate between: - unseeded hash for 32-bit integers - unseeded hash for 64-bit integers - seeded hash for 32-bit integers - seeded hash for strings R=bmeurer@chromium.org Bug: chromium:680662 Change-Id: I7459958c4158ee3501c962943dff8f33258bb5ce Reviewed-on: https://chromium-review.googlesource.com/1235973 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#56068} commit 95a979e02d7154e45b293261a6998c99d71fc238 Author: Yang Guo <yangguo@chromium.org> Date: Thu Sep 20 14:34:48 2018 Call into C++ to compute seeded integer hash R=bmeurer@chromium.org Bug: chromium:680662 Change-Id: I8dace89d576dfcc5833fd539ce698a9ade1cb5a0 Reviewed-on: https://chromium-review.googlesource.com/1235928 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#56091} commit 2c2af0022d5feb9e525a00a76cb15db9f3e38dba Author: Yang Guo <yangguo@chromium.org> Date: Thu Sep 27 16:37:57 2018 Use 64-bit for seeded integer hashes R=petermarshall@chromium.org Bug: chromium:680662 Change-Id: If48d1043dbe1e1bb695ec890c23e103a6cacf2d4 Reviewed-on: https://chromium-review.googlesource.com/1244220 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#56271} Refs: #23259 PR-URL: #23274 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

MylesBorins · 2018-10-31T06:14:27Z

landed in 7ab253f

Notable changes: * **assert**: - backport some assert commits (Ruben Bridgewater) [#23223](#23223) * **deps**: - V8: cherry-pick 64-bit hash seed commits (Yang Guo) [#23274](#23274) * **http**: - added aborted property to request (Robert Nagy) [#20094](#20094) * **http2**: - backport http2 changes from 10.x (Kelvin Jin) [#22850](#22850) PR-URL: #23974

Notable changes: * **assert**: - backport some assert commits (Ruben Bridgewater) [#23223](#23223) * **deps**: - upgrade to libuv 1.23.2 (cjihrig) [#23336](#23336) - V8: cherry-pick 64-bit hash seed commits (Yang Guo) [#23274](#23274) * **http**: - added aborted property to request (Robert Nagy) [#20094](#20094) * **http2**: - graduate from experimental (James M Snell) [#22466](#22466) PR-URL: #23974

Notable changes: * **assert**: - backport some assert commits (Ruben Bridgewater) [nodejs#23223](nodejs#23223) * **deps**: - upgrade to libuv 1.23.2 (cjihrig) [nodejs#23336](nodejs#23336) - V8: cherry-pick 64-bit hash seed commits (Yang Guo) [nodejs#23274](nodejs#23274) * **http**: - added aborted property to request (Robert Nagy) [nodejs#20094](nodejs#20094) * **http2**: - graduate from experimental (James M Snell) [nodejs#22466](nodejs#22466) PR-URL: nodejs#23974

hashseed requested review from mcollina, ofrobots, rvagg and targos October 5, 2018 10:38

hashseed requested a review from mhdawson October 5, 2018 10:38

nodejs-github-bot added v8 engine Issues and PRs related to the V8 dependency. v8.x labels Oct 5, 2018

hashseed force-pushed the 64bithash_on_8 branch from 6eb0c4c to a2e924e Compare October 5, 2018 10:40

hashseed changed the title ~~deps: V8: cherry-pick 64-bit hash seed commits~~ [8.x] deps: V8: cherry-pick 64-bit hash seed commits Oct 5, 2018

hashseed changed the title ~~[8.x] deps: V8: cherry-pick 64-bit hash seed commits~~ [v8.x] deps: V8: cherry-pick 64-bit hash seed commits Oct 5, 2018

addaleax approved these changes Oct 5, 2018

View reviewed changes

mcollina approved these changes Oct 6, 2018

View reviewed changes

hashseed force-pushed the 64bithash_on_8 branch 2 times, most recently from 1dbd688 to 71cea05 Compare October 22, 2018 09:59

This comment has been minimized.

Sign in to view

refack added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 22, 2018

refack removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 23, 2018

hashseed force-pushed the 64bithash_on_8 branch from 71cea05 to 7aef2d4 Compare October 23, 2018 18:58

refack added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 25, 2018

MylesBorins closed this Oct 31, 2018

MylesBorins added the notable-change PRs with changes that should be highlighted in changelogs. label Oct 31, 2018

BethGriggs mentioned this pull request Nov 12, 2018

v8.13.0 proposal #23974

Merged

hashseed deleted the 64bithash_on_8 branch November 20, 2018 08:15

beevelop mentioned this pull request Nov 20, 2018

2018-11-20 Version 8.13.0 'Carbon' (LTS) @MylesBorins prepared by @BethGriggs beevelop/docker-android-nodejs#250

Closed

Uh oh!

Conversation

hashseed commented Oct 5, 2018

Checklist

Uh oh!

nodejs-github-bot commented Oct 5, 2018

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

BethGriggs commented Oct 16, 2018

Uh oh!

This comment has been minimized.

hashseed commented Oct 22, 2018

Uh oh!

refack commented Oct 22, 2018

Uh oh!

refack commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

refack commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hashseed commented Oct 23, 2018

Uh oh!

hashseed commented Oct 23, 2018

Uh oh!

refack commented Oct 23, 2018

Uh oh!

hashseed commented Oct 25, 2018

Uh oh!

refack commented Oct 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BethGriggs commented Oct 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MylesBorins commented Oct 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

refack commented Oct 23, 2018 •

edited

Loading

refack commented Oct 23, 2018 •

edited

Loading

refack commented Oct 25, 2018 •

edited

Loading

BethGriggs commented Oct 30, 2018 •

edited

Loading