[v8.x] deps: V8: cherry-pick 64-bit hash seed commits #23274

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@{#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: 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 Oct 5, 2018

hashseed requested a review from mhdawson Oct 5, 2018

nodejs-github-bot added v8 engine v8.x labels Oct 5, 2018

hashseed force-pushed the 64bithash_on_8 branch from 6eb0c4c to a2e924e Compare Oct 5, 2018

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 changes

mcollina approved these changes Oct 6, 2018

View changes

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

This comment has been hidden.

Sign in to view

refack added the author ready label Oct 22, 2018

refack removed the author ready label Oct 23, 2018

hashseed force-pushed the 64bithash_on_8 branch from 71cea05 to 7aef2d4 Compare Oct 23, 2018

refack added the author ready label Oct 25, 2018

MylesBorins closed this Oct 31, 2018

MylesBorins added the notable-change 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 Nov 20, 2018

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

nodejs / node Public

[v8.x] deps: V8: cherry-pick 64-bit hash seed commits #23274

[v8.x] deps: V8: cherry-pick 64-bit hash seed commits #23274

hashseed commented Oct 5, 2018

nodejs-github-bot commented Oct 5, 2018

mcollina left a comment

BethGriggs commented Oct 16, 2018

This comment has been hidden.

hashseed commented Oct 22, 2018

refack commented Oct 22, 2018

refack commented Oct 23, 2018 •

edited

refack commented Oct 23, 2018 •

edited

hashseed commented Oct 23, 2018

hashseed commented Oct 23, 2018

refack commented Oct 23, 2018

hashseed commented Oct 25, 2018

refack commented Oct 25, 2018 •

edited

BethGriggs commented Oct 30, 2018 •

edited

MylesBorins commented Oct 31, 2018

nodejs / node Public

[v8.x] deps: V8: cherry-pick 64-bit hash seed commits #23274

[v8.x] deps: V8: cherry-pick 64-bit hash seed commits #23274

Conversation

hashseed commented Oct 5, 2018

Checklist

nodejs-github-bot commented Oct 5, 2018

mcollina left a comment

BethGriggs commented Oct 16, 2018

This comment has been hidden.

hashseed commented Oct 22, 2018

refack commented Oct 22, 2018

refack commented Oct 23, 2018 • edited

refack commented Oct 23, 2018 • edited

hashseed commented Oct 23, 2018

hashseed commented Oct 23, 2018

refack commented Oct 23, 2018

hashseed commented Oct 25, 2018

refack commented Oct 25, 2018 • edited

BethGriggs commented Oct 30, 2018 • edited

MylesBorins commented Oct 31, 2018

refack commented Oct 23, 2018 •

edited

refack commented Oct 23, 2018 •

edited

refack commented Oct 25, 2018 •

edited

BethGriggs commented Oct 30, 2018 •

edited