ComputeRandomMagic produces less randomness on 64-bit platforms than 32-bit platforms |
||||||||||
Issue descriptionBuild: GLinux, 64 bit. ToT (built at b2695df06e09e101a7ba302638978788a715c209) I found an issue when trying to enable warnings for 64 to 32 bit narrowing in blink and I believe there is a problem with the code that reduces effective randomness of ComputeRandomMagic (https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/heap/heap_page.cc?sq=package:chromium&g=0&l=1766) on 64 bit platforms. The problem I believe is with the truncation that happens into the RotateLeft16 that only takes a 32 bit value in. ie: const uintptr_t random2 = ~(RotateLeft16(reinterpret_cast<uintptr_t>(::read))); This line will always produce a value with the top high 4 bytes always being 0. Then this is shifted 32 bits in: const uint32_t random = static_cast<uint32_t>( (random1 & 0x0FFFFULL) | ((random2 >> 32) & 0x0FFFF0000ULL)); So really this is effectively: const uint32_t random = static_cast<uint32_t>( random1 & 0x0FFFFULL));
,
Jul 6
We should try to get the CL merged into 68 if possible, when it lands.
,
Jul 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2360107a8cde55e4e8024c07ca5c826f37b2e782 commit 2360107a8cde55e4e8024c07ca5c826f37b2e782 Author: Chris Palmer <palmer@chromium.org> Date: Sat Jul 07 15:38:58 2018 Don't throw away bits when computing Blink heap magic. Bug: 860721 Change-Id: I2b0c092d959e8ee1162d56f506fa7fa12d868887 Reviewed-on: https://chromium-review.googlesource.com/1128201 Commit-Queue: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Cr-Commit-Position: refs/heads/master@{#573166} [modify] https://crrev.com/2360107a8cde55e4e8024c07ca5c826f37b2e782/third_party/blink/renderer/platform/heap/heap_page.cc
,
Jul 9
,
Jul 9
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 9
,
Jul 9
Has the fix been verified in canary?
,
Jul 9
If there's been a canary build since #3, then yes. Looks like yes; my canary is Revision 472d1caeb99d99a8952e7170bbf435bd92902d73-refs/branch-heads/3486@{#1}, and 472d1caeb99d99a8952e7170bbf435bd92902d73 is from today.
,
Jul 10
,
Jul 10
Approved - branch:3440
,
Jul 10
Thanks, abdulsyed. Unfortunately, it doesn't merge cleanly: In 3440, the code is in heap_page.h instead of heap_page.cc, and is called GetRandomMagic instead of ComputeRandomMagic. Other than that, it's the same. So I'm uploading a CL for you to review: https://chromium-review.googlesource.com/c/chromium/src/+/1132082
,
Jul 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f38f1665b55e701025257e9b5bb901b0e7f96a7e commit f38f1665b55e701025257e9b5bb901b0e7f96a7e Author: Chris Palmer <palmer@chromium.org> Date: Wed Jul 11 00:01:33 2018 Don't throw away bits when computing Blink heap magic. Bug: 860721 Change-Id: I8d32e43efa102d257360c2886aa1701298e3c91e Reviewed-on: https://chromium-review.googlesource.com/1132082 Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#643} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/f38f1665b55e701025257e9b5bb901b0e7f96a7e/third_party/blink/renderer/platform/heap/heap_page.h
,
Jul 23
,
Oct 16
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by palmer@chromium.org
, Jul 6Labels: Security_Impact-Stable Security_Severity-Low OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Owner: palmer@chromium.org
Status: Started (was: Untriaged)
Summary: ComputeRandomMagic produces less randomness on 64-bit platforms than 32-bit platforms (was: ComputeRandomMagic produces less randomness on 64 bit platforms than 32 bit platforms)