Land JIT hardening nop insertion

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
7 years ago
6 months ago

People

(Reporter: cdleary, Unassigned)

Tracking

({parity-edge, sec-want})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Created attachment 599855 [details] [diff] [review]
0. Factor out a random number generator interface.

Initial RNG patch cleans up the kind of gross randomization reuse from the prior patch and makes a centralized RNG for JIT hardening in the JSRuntime that can be seeded at program startup.
Attachment #599855 - Flags: review?(dvander)
Attachment #599855 - Flags: review?(dvander) → review+
Created attachment 599859 [details] [diff] [review]
3. Shell option for seeding randomization.

Patches 1 and 2 were just rebased from dmandelin's reviews -- this lets us seed the hardening via a shell option for maximum reproducibility WRT try failures. I also fixed some of the uses of print-like-a-function in jit-tests.py because it bugged me. :-)
Attachment #599859 - Flags: review?(dvander)
Attachment #599859 - Flags: review?(dvander) → review+
Created attachment 600213 [details] [diff] [review]
2.5. Use the RNG as the hardening enabler.

Simple change, but different enough it probably needs an r+ to land.
Attachment #600213 - Flags: review?(dvander)
Comment on attachment 600213 [details] [diff] [review]
2.5. Use the RNG as the hardening enabler.

Review of attachment 600213 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/assembler/assembler/MacroAssemblerX86.h
@@ +220,5 @@
>  
>      Jump branchPtrWithPatch(Condition cond, RegisterID left, DataLabelPtr& dataLabel, ImmPtr initialRightValue = ImmPtr(0))
>      {
>          {
> +            AutoUnharden au(this);

Do these calls need to appear in relevant places in X64.h and the ARM macro assembler?
Attachment #600213 - Flags: review?(dvander) → review+
Mass-reassigning cdleary's bugs to default. He won't work on any of them, anymore. I guess, at least.

@cdleary: shout if you take issue with this.
Assignee: cdleary → general
Status: ASSIGNED → NEW
(Assignee)

Updated

4 years ago
Assignee: general → nobody
Keywords: sec-want
Blocks: 677272
No longer depends on: 677272

Comment 5

8 months ago
I don't think NOP Insertion is valuable enough to implement, as it can be bypassed readily. If anyone wants to debate about, feel free to comment or re-open.
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → WONTFIX

Comment 7

6 months ago
FWIW, we use nop insertion for fuzzing (as it tends to find bugs related to branch targeting, incorrectly not-fused code sequences, and so on); the ARM back-end implements support for it, though probably not elaborate enough for JIT hardening.
You need to log in before you can comment on or make changes to this bug.