Bug 159989 - Randomise the start of the jit copy function
Summary: Randomise the start of the jit copy function
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-07-20 15:33 PDT by Oliver Hunt
Modified: 2017-05-04 17:50 PDT (History)
7 users (show)

See Also:

Attachments
Patch (2.37 KB, patch)
2016-07-20 15:38 PDT, Oliver Hunt 		barraclough: review-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2016-07-20 15:33:43 PDT 
Randomise the start of the jit copy function
Comment 1 Oliver Hunt 2016-07-20 15:38:53 PDT 
Created attachment 284157 [details]
Patch
Comment 2 Gavin Barraclough 2016-07-20 16:06:30 PDT 
Comment on attachment 284157 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284157&action=review

> Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:280
> +        functionStartOffset &= 31;

This looks wrong – I think you're picking an unaligned point within the first 32 bytes

I think you want:
    functionStartOffset &= ~0x1F;
or even:
    functionStartOffset -= functionStartOffset & 0x1F;
Comment 3 Mark Lam 2016-07-20 16:15:16 PDT 
Comment on attachment 284157 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284157&action=review

>> Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:280
>> +        functionStartOffset &= 31;
> 
> This looks wrong – I think you're picking an unaligned point within the first 32 bytes
> 
> I think you want:
>     functionStartOffset &= ~0x1F;
> or even:
>     functionStartOffset -= functionStartOffset & 0x1F;

Yes, this was looking wrong for me too.  You definitely want:
    functionStartOffset &= ~0x1F;
Comment 4 Alexey Proskuryakov 2016-07-20 18:54:42 PDT 
Comment on attachment 284157 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284157&action=review

> Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:277
> +        size_t functionStartOffset = WTF::cryptographicallyRandomNumber() % stubExtent;

Please do not add a WTF:: prefix.

> Source/JavaScriptCore/jit/ExecutableAllocatorFixedVMPool.cpp:284
> +        LinkBuffer linkBuffer(jit, static_cast<char *>(stubBase) + functionStartOffset, stubSize - functionStartOffset);

Misplaced star.
Comment 5 David Kilzer (:ddkilzer) 2017-05-04 17:50:48 PDT 
<rdar://problems/32004913>