This is an archive of the discontinued LLVM Phabricator instance.

Randomize instruction scheduling to increase security against ROP attacks (llvm)
AbandonedPublic

Authored by yln on Apr 15 2014, 11:07 PM.

Details

Reviewers
ahomescu
rinon
Summary

Modifies the selection DAG scheduler to randomize instruction selection to introduce diversity with the goal of increasing security against most return-oriented programming attacks.

Command line options:

-sched-randomize // Enable the schedule randomizer.
-sched-randomize-percentage=X // X% of instructions are scheduled randomly (default: 50%, requires -sched-randomize).

This is the llvm part of the patch.
clang part: D3395

Diff Detail

Event Timeline

jfb added inline comments.Apr 17 2014, 4:16 PM
lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
115

Same as other review: cl::cat, and check that this isn't over 100%.

Actually why would you ever set scheduler randomization to less than 100%? IMO you should either randomize all-out, or not at all. Could you just drop the percentage from this change?

1773

This will access thread-local storage each time you try to insert a NOP. Could you instead access the RNG once in the class' construction, so you don't go through TLS?

1804

Same on TLS.

test/CodeGen/X86/sched-rnd-test.ll
3

I think you need to specify a target? This test won't work if x86 isn't the default.

18

Do you think that it would be possible to test that dependencies between instructions are respected? Say with a function that has a long dependency chain so that statistically if there were a bug you'd see at least one invalid swap?

rinon added inline comments.Apr 18 2014, 11:28 AM
lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
115

We intended this to be a knob to adjust a performance vs security tradeoff. In order to make sure that we don't affect performance too much, we wanted to set a lower default, and allow users to up this if they wish.

jfb added inline comments.Apr 18 2014, 11:49 AM
lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
115

I see. Once this change is further along could you quantify the performance difference, and add those numbers to the commit message? I.e. run SPEC2k or something with a few randomization percentages, and figure out what's in the noise. This could also help choose an acceptable default for the option, where the default gives a schedule randomization that's statistically equivalent to general benchmarking noise.

yln abandoned this revision.Aug 15 2017, 4:08 PM