lkml.org 
[lkml]   [2022]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch 00/38] x86/retbleed: Call depth tracking mitigation
    On Thu, Jul 21, 2022 at 05:16:14PM -0700, Sami Tolvanen wrote:

    > That looks good to me. I updated my LLVM tree to generate this code
    > for the checks:
    >
    > https://github.com/samitolvanen/llvm-project/commits/kcfi

    Thanks!

    The alignment thing you added:

    // Emit int3 padding before the type information to maintain alignment.
    // The X86::MOV32ri instruction we emit is 5 bytes long.
    uint64_t Padding = offsetToAlignment(5, MF.getAlignment());
    while (Padding--)
    EmitAndCountInstruction(MCInstBuilder(X86::INT3));

    Doesn't seem to quite do what we want though.

    When I use -fpatchable-function-entry=16,16 we effectively get a 32 byte
    prefix on every function:

    0000000000000000 <__cfi___traceiter_sched_kthread_stop>:
    0: cc int3
    1: cc int3
    2: cc int3
    3: cc int3
    4: cc int3
    5: cc int3
    6: cc int3
    7: cc int3
    8: cc int3
    9: cc int3
    a: cc int3
    b: b8 26 b1 df 98 mov $0x98dfb126,%eax
    10: 90 nop
    11: 90 nop
    12: 90 nop
    13: 90 nop
    14: 90 nop
    15: 90 nop
    16: 90 nop
    17: 90 nop
    18: 90 nop
    19: 90 nop
    1a: 90 nop
    1b: 90 nop
    1c: 90 nop
    1d: 90 nop
    1e: 90 nop
    1f: 90 nop

    And given the parameters, that's indeed the only option. However, given
    I can scribble the type thing just fine when moving to FineIBT and the
    whole Skylake depth tracking only needs 10 bytes, I figured I'd try:
    -fpatchable-function-entry=11,11 instead. But that resulted in
    unalignment:

    0000000000000000 <__cfi___traceiter_sched_kthread_stop>:
    0: cc int3
    1: cc int3
    2: cc int3
    3: cc int3
    4: cc int3
    5: cc int3
    6: cc int3
    7: cc int3
    8: cc int3
    9: cc int3
    a: cc int3
    b: b8 26 b1 df 98 mov $0x98dfb126,%eax
    10: 90 nop
    11: 90 nop
    12: 90 nop
    13: 90 nop
    14: 90 nop
    15: 90 nop
    16: 90 nop
    17: 90 nop
    18: 90 nop
    19: 90 nop
    1a: 90 nop

    000000000000001b <__traceiter_sched_kthread_stop>:

    However, if I change clang like so:

    llvm/lib/Target/X86/X86AsmPrinter.cpp | 8 +++++++-
    1 file changed, 7 insertions(+), 1 deletion(-)

    diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp
    index 789597f8ef1a..6c94313a197d 100644
    --- a/llvm/lib/Target/X86/X86AsmPrinter.cpp
    +++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp
    @@ -124,9 +124,15 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF,
    OutStreamer->emitSymbolAttribute(FnSym, MCSA_ELF_TypeFunction);
    OutStreamer->emitLabel(FnSym);

    + int64_t PrefixNops = 0;
    + (void)MF.getFunction()
    + .getFnAttribute("patchable-function-prefix")
    + .getValueAsString()
    + .getAsInteger(10, PrefixNops);
    +
    // Emit int3 padding before the type information to maintain alignment.
    // The X86::MOV32ri instruction we emit is 5 bytes long.
    - uint64_t Padding = offsetToAlignment(5, MF.getAlignment());
    + uint64_t Padding = offsetToAlignment(5+PrefixNops, MF.getAlignment());
    while (Padding--)
    EmitAndCountInstruction(MCInstBuilder(X86::INT3));

    Then it becomes:

    0000000000000000 <__cfi___traceiter_sched_kthread_stop>:
    0: b8 26 b1 df 98 mov $0x98dfb126,%eax
    5: 90 nop
    6: 90 nop
    7: 90 nop
    8: 90 nop
    9: 90 nop
    a: 90 nop
    b: 90 nop
    c: 90 nop
    d: 90 nop
    e: 90 nop
    f: 90 nop
    0000000000000010 <__traceiter_sched_kthread_stop>:
    and things are 'good' again, except for functions that don't get a kcfi
    preamble, those are unaligned... I couldn't find where the
    patchable-function-prefix nops are generated to fix this up :/


    Also; could you perhaps add a switch to supress ENDBR for functions with
    a kCFI preamble ?

    \
     
     \ /
      Last update: 2022-07-22 12:25    [W:3.314 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site