Attachment 347474 Details for Bug 188556 – Patch

[patch] Patch

bug-188556-20180820100321.patch (text/plain), 2.85 KB, created by Yusuke Suzuki on 2018-08-19 18:03:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-08-19 18:03:22 PDT

Size: 2.85 KB

patch

obsolete

>Subversion Revision: 235022
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 840cdad63060329ff53147998d5408d5d9bf6449..17c0db3c61e1c9fda6d43d8c5463d43abcbbaba7 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-08-19  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [JSC] Should not rotate constant with 64
>+        https://bugs.webkit.org/show_bug.cgi?id=188556
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        To defend against JIT splaying, we rotate a constant with a randomly generated seed.
>+        But if a seed becomes 64 or 0, the following code performs `value << 64` or `value >> 64`
>+        where value's type is uint64_t, and they cause undefined behaviors (UBs). This patch limits
>+        the seed in the range of [1, 63] not to generate code causing UBs. This is found by UBSan.
>+
>+        * assembler/MacroAssembler.h:
>+        (JSC::MacroAssembler::generateRotationSeed):
>+        (JSC::MacroAssembler::rotationBlindConstant):
>+
> 2018-08-19  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r234852.
>diff --git a/Source/JavaScriptCore/assembler/MacroAssembler.h b/Source/JavaScriptCore/assembler/MacroAssembler.h
>index adbac90c7beab53398d17a6fbae83255470c8587..a1edb9be734cd3c138b91603c776b85efcaeabe2 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssembler.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssembler.h
>@@ -1290,6 +1290,13 @@ class MacroAssembler : public MacroAssemblerBase {
> 
>         return shouldBlindPointerForSpecificArch(value);
>     }
>+
>+    uint8_t generateRotationSeed(size_t widthInBits)
>+    {
>+        // Generate the seed in [1, widthInBits - 1]. We should not generate widthInBits or 0
>+        // since it leads to `<< widthInBits` or `>> widthInBits`, which cause undefined behaviors.
>+        return (random() % (widthInBits - 1)) + 1;
>+    }
>     
>     struct RotatedImmPtr {
>         RotatedImmPtr(uintptr_t v1, uint8_t v2)
>@@ -1303,7 +1310,7 @@ class MacroAssembler : public MacroAssemblerBase {
>     
>     RotatedImmPtr rotationBlindConstant(ImmPtr imm)
>     {
>-        uint8_t rotation = random() % (sizeof(void*) * 8);
>+        uint8_t rotation = generateRotationSeed(sizeof(void*) * 8);
>         uintptr_t value = imm.asTrustedImmPtr().asIntptr();
>         value = (value << rotation) | (value >> (sizeof(void*) * 8 - rotation));
>         return RotatedImmPtr(value, rotation);
>@@ -1371,7 +1378,7 @@ class MacroAssembler : public MacroAssemblerBase {
>     
>     RotatedImm64 rotationBlindConstant(Imm64 imm)
>     {
>-        uint8_t rotation = random() % (sizeof(int64_t) * 8);
>+        uint8_t rotation = generateRotationSeed(sizeof(int64_t) * 8);
>         uint64_t value = imm.asTrustedImm64().m_value;
>         value = (value << rotation) | (value >> (sizeof(int64_t) * 8 - rotation));
>         return RotatedImm64(value, rotation);

Flags:

saam: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 188556: 347080 | 347474 | 347478