Messages in this thread Patch in this message | | | Date | Thu, 21 Jul 2022 19:55:55 +0200 | From | Peter Zijlstra <> | Subject | Re: [patch 00/38] x86/retbleed: Call depth tracking mitigation |
| |
On Thu, Jul 21, 2022 at 05:54:38PM +0200, Peter Zijlstra wrote: > My very firstest LLVM patch; except it explodes at runtime and I'm not > sure where to start looking... > > On top of sami-llvm/kcfi
Thanks Sami!
this seems to work, let me go hack the kernel..
--- clang/lib/Driver/SanitizerArgs.cpp | 12 --------- llvm/lib/Target/X86/X86AsmPrinter.cpp | 11 -------- llvm/lib/Target/X86/X86MCInstLower.cpp | 47 ++++++++++++++++++++++------------ 3 files changed, 31 insertions(+), 39 deletions(-)
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 373a74399df0..b6ebc8ad1842 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -719,18 +719,6 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, D.Diag(diag::err_drv_argument_not_allowed_with) << "-fsanitize=kcfi" << lastArgumentForMask(D, Args, SanitizerKind::CFI); - - if (Arg *A = Args.getLastArg(options::OPT_fpatchable_function_entry_EQ)) { - StringRef S = A->getValue(); - unsigned N, M; - // With -fpatchable-function-entry=N,M, where M > 0, - // llvm::AsmPrinter::emitFunctionHeader injects nops before the - // KCFI type identifier, which is currently unsupported. - if (!S.consumeInteger(10, N) && S.consume_front(",") && - !S.consumeInteger(10, M) && M > 0) - D.Diag(diag::err_drv_argument_not_allowed_with) - << "-fsanitize=kcfi" << A->getAsString(Args); - } } Stats = Args.hasFlag(options::OPT_fsanitize_stats, diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp index 5e011d409ee8..ffdb95324da7 100644 --- a/llvm/lib/Target/X86/X86AsmPrinter.cpp +++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp @@ -124,23 +124,12 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF, OutStreamer->emitSymbolAttribute(FnSym, MCSA_ELF_TypeFunction); OutStreamer->emitLabel(FnSym); - // Emit int3 padding to allow runtime patching of the preamble. - EmitAndCountInstruction(MCInstBuilder(X86::INT3)); - EmitAndCountInstruction(MCInstBuilder(X86::INT3)); - // Embed the type hash in the X86::MOV32ri instruction to avoid special // casing object file parsers. EmitAndCountInstruction(MCInstBuilder(X86::MOV32ri) .addReg(X86::EAX) .addImm(Type->getZExtValue())); - // The type hash is encoded in the last four bytes of the X86::MOV32ri - // instruction. Emit additional X86::INT3 padding to ensure the hash is - // at offset -6 from the function start to avoid potential call target - // gadgets in checks emitted by X86AsmPrinter::LowerKCFI_CHECK. - EmitAndCountInstruction(MCInstBuilder(X86::INT3)); - EmitAndCountInstruction(MCInstBuilder(X86::INT3)); - if (MAI->hasDotTypeDotSizeDirective()) { MCSymbol *EndSym = OutContext.createTempSymbol("cfi_func_end"); OutStreamer->emitLabel(EndSym); diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp index 16c4d2e45970..4ed23348aa7c 100644 --- a/llvm/lib/Target/X86/X86MCInstLower.cpp +++ b/llvm/lib/Target/X86/X86MCInstLower.cpp @@ -1340,22 +1340,37 @@ void X86AsmPrinter::LowerKCFI_CHECK(const MachineInstr &MI) { assert(std::next(MI.getIterator())->isCall() && "KCFI_CHECK not followed by a call instruction"); - // The type hash is encoded in the last four bytes of the X86::CMP32mi - // instruction. If we decided to place the hash immediately before - // indirect call targets (offset -4), the X86::JCC_1 instruction we'll - // emit next would be a potential indirect call target as it's preceded - // by a valid type hash. - // - // To avoid generating useful gadgets, X86AsmPrinter::emitKCFITypeId - // emits the type hash prefix at offset -6, which makes X86::TRAP the - // only possible target in this instruction sequence. - EmitAndCountInstruction(MCInstBuilder(X86::CMP32mi) - .addReg(MI.getOperand(0).getReg()) - .addImm(1) - .addReg(X86::NoRegister) - .addImm(-6) - .addReg(X86::NoRegister) - .addImm(MI.getOperand(1).getImm())); + const Function &F = MF->getFunction(); + unsigned Imm = MI.getOperand(1).getImm(); + unsigned Num = 0; + + if (F.hasFnAttribute("patchable-function-prefix")) { + if (F.getFnAttribute("patchable-function-prefix") + .getValueAsString() + .getAsInteger(10, Num)) + Num = 0; + } + + // movl $(~0x12345678), %r10d + EmitAndCountInstruction(MCInstBuilder(X86::MOV32ri) + .addReg(X86::R10D) // dst + .addImm(~Imm)); + + // negl %r10d + EmitAndCountInstruction(MCInstBuilder(X86::NEG32r) + .addReg(X86::R10D) // dst + .addReg(X86::R10D) // src + ); + + // cmpl %r10d, -off(%r11) + EmitAndCountInstruction(MCInstBuilder(X86::CMP32mr) + .addReg(MI.getOperand(0).getReg()) // base + .addImm(0) // scale + .addReg(0) // index + .addImm(-(Num+4)) // offset + .addReg(0) // segment + .addReg(X86::R10D) // reg + ); MCSymbol *Pass = OutContext.createTempSymbol(); EmitAndCountInstruction(
| |