Page MenuHomePhabricator

[AArch64] Add BTI to CFI jumptables.
AcceptedPublic

Authored by danielkiss on Jun 5 2020, 4:28 AM.

Details

Summary

With branch protection the jump to the jump table entries requires a landing pad.

Diff Detail

Event Timeline

danielkiss created this revision.Jun 5 2020, 4:28 AM

This patch depends on D80791 and D75181.

eugenis added inline comments.Jun 5 2020, 11:26 AM
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1220

I'd rather this was a named constant.

1242

Do we need to check the function attribute here as well? What happens if a function opts out of BTI?

danielkiss marked 2 inline comments as done.Jun 8 2020, 2:10 AM
danielkiss added inline comments.
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1220

I'll update it.

1242

No, direct jump does not need a landing pad. So this jump will be fine even that target function opted out.

danielkiss updated this revision to Diff 269251.Jun 8 2020, 8:36 AM
danielkiss marked 2 inline comments as done.
eugenis accepted this revision.Jun 8 2020, 5:05 PM

LGTM

llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1242

What I meant is, if a function has ignore-branch-target-enforcement, do not we want to skip the BTI hint in that function's CFI stub?
Or, rather, replace it with NOP to preserve the stub size.

Thinking about it some more - no, probably not. The attribute can be used on a "naked" function that already has BTI in the assembly; in that case we'd need BTI in the jump table as well.

This revision is now accepted and ready to land.Jun 8 2020, 5:05 PM
danielkiss planned changes to this revision.Sat, Aug 29, 6:37 AM

I need to update it due to changes in the dependent patches.

chill added inline comments.Tue, Sep 1, 4:38 AM
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1406

Why are we disabling BTI here?

1407

Here we should use sign-return-address=none, and, if needed, I suggest (re-)introducing branch-target-enforcement=true|false ?

danielkiss added inline comments.Tue, Sep 1, 4:59 AM
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1406

This function is a jump table and each entry will have a landing pad ( see line 1235).
If it is not disabled then the entry will get an additional landing pad that miss-aligns the table.

Worst case we could add special condition for the first entry but IMHO that would be ugly.

1407

Agree , I will update once the D85649 is updated as proposed there.

danielkiss marked an inline comment as not done.Wed, Sep 16, 1:52 PM
danielkiss added inline comments.
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1243

since D81257 we can use bti c in assembly.

rebasing to the new patch series.

This revision is now accepted and ready to land.Wed, Sep 16, 1:56 PM